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é <berra...@redhat.com> > Signed-off-by: Richard Henderson <richard.hender...@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é <berra...@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 :|