Hi Kevin,
On 2026-04-23T14:10:31+0800, Kevin J. McCarthy wrote:
> +/* Generate length_requested random bytes of data */
> +void mutt_random_bytes(char *random_bytes, size_t length_requested)
> +{
> +#if defined(HAVE_GETRANDOM)
> + size_t res;
> +
> + do
> + {
> + res = getrandom(random_bytes, length_requested, GRND_NONBLOCK);Do we really want to not block? Even on ancient systems on which it may block, I expect one can wait a few ms (or seconds) until the entropy has been gathered. That would keep the source code simpler. Or is this used frequently enough that it would slow down the program to unreasonable times? > + } while ((res == (size_t) -1) && (errno == EINTR)); This cast is dangerous. It's safer to compare to literal -1. I know you'll get a -Wsign-compare (part of -Wextra) diagnostic, but that's something that GCC should fix. I've been using -Wno-error=sign-compare for some time, precisely for the false negatives. If for some reason, the type of the cast doesn't match the type of the variable, we'll get a bug. On the other hand, non-casted literal -1 will magically convert to any wider unsigned type and do the right thing. See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119011>. > + > + if ((res == (size_t) -1) || (res < length_requested)) Same thing about the cast. > + { > + if (res < length_requested) > + { > + length_requested -= res; > + random_bytes += res; Maybe prng_random_bytes() should get a pointer to the end of the array instead of a length, so that it doesn't need to be updated? The idea is similar to Plan9's strecpy(2) string-copying function, documented here: <http://man.cat-v.org/plan_9_3rd_ed/2/strcat> Other than these minor comments, the patch seems reasonable to me. Have a lovely day! Alex > + } > + prng_random_bytes(random_bytes, length_requested); > + } > +#elif defined(HAVE_ARC4RANDOM_BUF) > + arc4random_buf(random_bytes, length_requested); > +#else > + prng_random_bytes(random_bytes, length_requested); > +#endif > +} > > /* Generate and Base64 encode 96 random bits and fill the buffer > output_B64 with the result. */ > diff --git a/mutt_random.h b/mutt_random.h > index ed9fdf29..0417e0d6 100644 > --- a/mutt_random.h > +++ b/mutt_random.h > @@ -25,6 +25,5 @@ typedef union random64 > } RANDOM64; > > void mutt_base64_random96(char output_B64[static 17]); > -void mutt_random_bytes(char *random_bytes, int length_requested); > -void mutt_reseed(void); > +void mutt_random_bytes(char *random_bytes, size_t length_requested); > #endif > -- > 2.53.0 > -- <https://www.alejandro-colomar.es>
signature.asc
Description: PGP signature
