[libvirt] [PATCH REPOST 2/4] storage: Use virRandomBytes for virStorageGenerateQcowPassphrase

[Ján Tomko]

On Mon, Jun 06, 2016 at 02:13:47PM -0400, John Ferlan wrote:
> Use the common API
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend.c   | 10 +++++++---
>  src/util/virstorageencryption.c | 42 +++++++++++++++--------------------------
>  src/util/virstorageencryption.h |  4 ++--
>  3 files changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 8f03a6e..fd432c8 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -597,7 +597,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
>      virStorageEncryptionSecretPtr enc_secret = NULL;
>      virSecretPtr secret = NULL;
>      char *xml;
> -    unsigned char value[VIR_STORAGE_QCOW_PASSPHRASE_SIZE];
> +    unsigned char *value = NULL;

On a 64-bit system, the original array is only twice the size,
I don't think switching to an allocated buffer is worth it.

>      int ret = -1;
>  
>      if (conn->secretDriver == NULL ||
> @@ -641,10 +641,13 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
>      }
>      VIR_FREE(xml);
>  
> -    if (virStorageGenerateQcowPassphrase(value) < 0)
> +    if (!(value =
> +          virStorageGenerateQcowPassphrase(VIR_STORAGE_QCOW_PASSPHRASE_SIZE)))
>          goto cleanup;
>  
> -    if (conn->secretDriver->secretSetValue(secret, value, sizeof(value), 0) < 0)
> +    if (conn->secretDriver->secretSetValue(secret, value,
> +                                           VIR_STORAGE_QCOW_PASSPHRASE_SIZE,
> +                                           0) < 0)
>          goto cleanup;
>  
>      enc_secret->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE;
> @@ -666,6 +669,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
>      virBufferFreeAndReset(&buf);
>      virSecretDefFree(def);
>      VIR_FREE(enc_secret);
> +    VIR_FREE(value);
>      return ret;
>  }
>  
> diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c
> index 8105158..00d1ff7 100644
> --- a/src/util/virstorageencryption.c
> +++ b/src/util/virstorageencryption.c
> @@ -1,7 +1,7 @@
>  /*
>   * virstorageencryption.c: volume encryption information
>   *
> - * Copyright (C) 2009-2014 Red Hat, Inc.
> + * Copyright (C) 2009-2014, 2016 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -34,6 +34,7 @@
>  #include "virerror.h"
>  #include "viruuid.h"
>  #include "virfile.h"
> +#include "virrandom.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_STORAGE
>  
> @@ -284,36 +285,23 @@ virStorageEncryptionFormat(virBufferPtr buf,
>      return 0;
>  }
>  
> -int
> -virStorageGenerateQcowPassphrase(unsigned char *dest)
> +unsigned char *
> +virStorageGenerateQcowPassphrase(size_t nbytes)

The length is already implied by the function name.

Also, since it's so specific and short, I would rather open-code it.

Jan