[libvirt] [PATCH REPOST 1/4] util: Add range parameters to virRandomBytes

[Ján Tomko]

On Mon, Jun 06, 2016 at 02:13:46PM -0400, John Ferlan wrote:
> Add a minval and maxval range for acceptible values from /dev/urandom
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/util/vircrypto.c     |  2 +-
>  src/util/virrandom.c     | 13 ++++++++++---
>  src/util/virrandom.h     |  3 ++-
>  src/util/viruuid.c       |  2 +-
>  tests/qemuxml2argvmock.c |  2 +-
>  tests/vircryptotest.c    |  4 ++--
>  tests/virrandommock.c    | 10 +++++++---
>  tests/virrandomtest.c    | 32 ++++++++++++++++++++++++--------
>  8 files changed, 48 insertions(+), 20 deletions(-)
> 
> diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c
> index 4f288f0..4125230 100644
> --- a/src/util/vircrypto.c
> +++ b/src/util/vircrypto.c
> @@ -301,7 +301,7 @@ virCryptoGenerateRandom(size_t nbytes)
>      /* If we don't have gnutls_rnd(), we will generate a less cryptographically
>       * strong master buf from /dev/urandom.
>       */
> -    if ((ret = virRandomBytes(buf, nbytes)) < 0) {
> +    if ((ret = virRandomBytes(buf, nbytes, 0x00, 0xff)) < 0) {

Currently, virRandomBytes never returns a negative value.

>          virReportSystemError(ret, "%s", _("failed to generate byte stream"));
>          VIR_FREE(buf);
>          return NULL;
> diff --git a/src/util/virrandom.c b/src/util/virrandom.c
> index 62a0e31..256819a 100644
> --- a/src/util/virrandom.c
> +++ b/src/util/virrandom.c
> @@ -164,13 +164,17 @@ uint32_t virRandomInt(uint32_t max)
>   * virRandomBytes
>   * @buf: Pointer to location to store bytes
>   * @buflen: Number of bytes to store
> + * @minval: Minimum value acceptable
> + * @maxval: Minimum value acceptable
>   *
>   * Generate a stream of random bytes from /dev/urandom
>   * into @buf of size @buflen
>   */
>  int
>  virRandomBytes(unsigned char *buf,
> -               size_t buflen)
> +               size_t buflen,
> +               uint8_t minval,
> +               uint8_t maxval)

Calling this, for example, virRandomBytesRange and making virRandomBytes
a macro with 0, 0xFF would look nicer for the callers using the usual
definition of bytes.

>  {
>      int fd;
>  
> diff --git a/tests/virrandommock.c b/tests/virrandommock.c
> index 6df5e20..d988bab 100644
> --- a/tests/virrandommock.c
> +++ b/tests/virrandommock.c
> @@ -28,12 +28,16 @@
>  
>  int
>  virRandomBytes(unsigned char *buf,
> -               size_t buflen)
> +               size_t buflen,
> +               uint8_t minval,
> +               uint8_t maxval)
>  {
>      size_t i;
>  
> -    for (i = 0; i < buflen; i++)
> -        buf[i] = i;
> +    for (i = 0; i < buflen; i++) {
> +        if (i >= minval && i <= maxval)
> +            buf[i] = i;
> +    }

This will leave everything up to buf[minval] untouched, which
is not what I would expect from real virRandomBytes...

>  
>      return 0;
>  }
> diff --git a/tests/virrandomtest.c b/tests/virrandomtest.c
> index 367bdc7..be148d2 100644
> --- a/tests/virrandomtest.c
> +++ b/tests/virrandomtest.c
> @@ -29,27 +29,31 @@
>  
>  # define VIR_FROM_THIS VIR_FROM_NONE
>  
> +struct testRandomData {
> +    uint8_t minval;
> +    uint8_t maxval;
> +};
> +
>  static int
> -testRandomBytes(const void *unused ATTRIBUTE_UNUSED)
> +testRandomBytes(const void *opaque)
>  {
>      int ret = -1;
>      size_t i;
>      uint8_t *data;
>      size_t datalen = 32;
> +    const struct testRandomData *range = opaque;
>  
>      if (VIR_ALLOC_N(data, datalen) < 0)
>          return -1;
>  
> -    if (virRandomBytes(data, datalen) < 0) {
> +    if (virRandomBytes(data, datalen, range->minval, range->maxval) < 0) {
>          fprintf(stderr, "Failed to generate random bytes");
>          goto cleanup;
>      }

...but since the only function this test is supposed to test is mocked,
this whole file can just be deleted.

virRandomBytes is basically reading from a file, I don't think there's
much to test there.

Jan