diff mbox series

[v2,03/14] crypto: Do not fail for EINTR during qcrypto_random_bytes

Message ID 20190314045526.13342-4-richard.henderson@linaro.org
State New
Headers show
Series Add qemu_getrandom and ARMv8.5-RNG | expand

Commit Message

Richard Henderson March 14, 2019, 4:55 a.m. UTC
We can always get EINTR for read; /dev/urandom is no exception.
Clean up return paths and avoid unnecessary goto.

Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 crypto/random-platform.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

-- 
2.17.1

Comments

Daniel P. Berrangé March 14, 2019, 3:34 p.m. UTC | #1
On Wed, Mar 13, 2019 at 09:55:15PM -0700, Richard Henderson wrote:
> We can always get EINTR for read; /dev/urandom is no exception.

> Clean up return paths and avoid unnecessary goto.

> 

> Cc: Daniel P. Berrangé <berrange@redhat.com>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  crypto/random-platform.c | 33 +++++++++++++--------------------

>  1 file changed, 13 insertions(+), 20 deletions(-)

> 

> diff --git a/crypto/random-platform.c b/crypto/random-platform.c

> index f995fc0ef1..0866f216dc 100644

> --- a/crypto/random-platform.c

> +++ b/crypto/random-platform.c

> @@ -65,29 +65,22 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,

>                           "Unable to read random bytes");

>          return -1;

>      }

> -

> -    return 0;

>  #else

> -    int ret = -1;

> -    int got;

> -

>      while (buflen > 0) {

> -        got = read(fd, buf, buflen);

> -        if (got < 0) {

> -            error_setg_errno(errp, errno,

> -                             "Unable to read random bytes");

> -            goto cleanup;

> -        } else if (!got) {

> -            error_setg(errp,

> -                       "Unexpected EOF reading random bytes");

> -            goto cleanup;

> +        ssize_t got = read(fd, buf, buflen);

> +        if (unlikely(got <= 0)) {


Not seeing a the need for the unlikely() call there - I'd
expect any benefit is dwarved by the read syscall time.

> +            if (got == 0) {

> +                error_setg(errp, "Unexpected EOF reading random bytes");

> +                return -1;

> +            } else if (errno != EINTR) {

> +                error_setg_errno(errp, errno, "Unable to read random bytes");

> +                return -1;

> +            }

> +        } else {

> +            buflen -= got;

> +            buf += got;

>          }

> -        buflen -= got;

> -        buf += got;

>      }

> -

> -    ret = 0;

> - cleanup:

> -    return ret;

>  #endif

> +    return 0;

>  }


Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
diff mbox series

Patch

diff --git a/crypto/random-platform.c b/crypto/random-platform.c
index f995fc0ef1..0866f216dc 100644
--- a/crypto/random-platform.c
+++ b/crypto/random-platform.c
@@ -65,29 +65,22 @@  int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
                          "Unable to read random bytes");
         return -1;
     }
-
-    return 0;
 #else
-    int ret = -1;
-    int got;
-
     while (buflen > 0) {
-        got = read(fd, buf, buflen);
-        if (got < 0) {
-            error_setg_errno(errp, errno,
-                             "Unable to read random bytes");
-            goto cleanup;
-        } else if (!got) {
-            error_setg(errp,
-                       "Unexpected EOF reading random bytes");
-            goto cleanup;
+        ssize_t got = read(fd, buf, buflen);
+        if (unlikely(got <= 0)) {
+            if (got == 0) {
+                error_setg(errp, "Unexpected EOF reading random bytes");
+                return -1;
+            } else if (errno != EINTR) {
+                error_setg_errno(errp, errno, "Unable to read random bytes");
+                return -1;
+            }
+        } else {
+            buflen -= got;
+            buf += got;
         }
-        buflen -= got;
-        buf += got;
     }
-
-    ret = 0;
- cleanup:
-    return ret;
 #endif
+    return 0;
 }