Re: [PATCH v6 03/10] benchtests: Add arc4random benchtest

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella via Libc-alpha:

> diff --git a/benchtests/bench-arc4random.c b/benchtests/bench-arc4random.c
> new file mode 100644
> index 0000000000..626f2ba48c
> --- /dev/null
> +++ b/benchtests/bench-arc4random.c
> @@ -0,0 +1,224 @@

> +/* Prevent compiler to optimize away call.  */
> +#define DO_NOT_OPTIMIZE(__value)		\
> +  ({						\
> +    __typeof (__value) __v = (__value);		\
> +    asm volatile("" : : "r,m" (__v) : "memory");\
> +  })

Missing space after “volatile”.

Maybe this should be a generic building block for benchtests?

> +static void
> +run_bench (json_ctx_t *json_ctx, const char *name,
> +	   char *const*fnames, size_t fnameslen,
> +	   void (*bench)(json_ctx_t *ctx))

Missing space between “)(”.


> diff --git a/stdlib/arc4random.c b/stdlib/arc4random.c
> index f6553dfd7d..74c917b9b9 100644
> --- a/stdlib/arc4random.c
> +++ b/stdlib/arc4random.c
> @@ -25,9 +25,9 @@
>  #include <sys/param.h>
>  #include <sys/random.h>
>  
> -/* Besides the cipher state 'ctx', it keeps two counters: 'have' is the
> -   current valid bytes not yet consumed in 'buf', while 'count' is the maximum
> -   number of bytes until a reseed.
> +/* arc4random keeps two counters: 'have' is the current valid bytes not yet
> +   consumed in 'buf' while 'count' is the maximum number of bytes until a
> +   reseed.
>  
>     Both the initial seed and reseed try to obtain entropy from the kernel
>     and abort the process if none could be obtained.
> @@ -81,10 +81,10 @@ arc4random_getrandom_failure (void)
>    __libc_fatal ("Fatal glibc error: Cannot get entropy for arc4random\n");
>  }
>  
> -/* Fork detection is done by checking if MADV_WIPEONFORK supported.  If not
> -   the fork callback will reset the state on the fork call.  It does not
> -   handle direct clone calls, nor vfork or _Fork (arc4random is not
> -   async-signal-safe due the internal lock usage).  */
> +/* If the kernel supports MADV_WIPEONFORK it is used to provide fork
> +   detection. Otherwise, the state is resetied with an atfork handler. The
> +   latter does not handle direct clone calls, nor vfork or _Fork (arc4random
> +   is not async-signal-safe due to the internal lock usage).  */
>  static void
>  arc4random_init (uint8_t *buf)
>  {

I think this belongs into some other patch.

Thanks,
Florian