Re: [PATCH v4 1/3] block/rbd: encryption nit fixes

[Daniel P . Berrangé]

On Sun, Nov 20, 2022 at 04:28:34AM -0600, Or Ozeri wrote:
> Add const modifier to passphrases,
> and remove redundant stack variable passphrase_len.
> 
> Signed-off-by: Or Ozeri <oro@il.ibm.com>
> ---
>  block/rbd.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index f826410f40..e575105e6d 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -330,7 +330,7 @@ static int qemu_rbd_set_keypairs(rados_t cluster, const 
> char *keypairs_json,
>  #ifdef LIBRBD_SUPPORTS_ENCRYPTION
>  static int qemu_rbd_convert_luks_options(
>          RbdEncryptionOptionsLUKSBase *luks_opts,
> -        char **passphrase,
> +        const char **passphrase,
>          size_t *passphrase_len,
>          Error **errp)
>  {
> @@ -341,7 +341,7 @@ static int qemu_rbd_convert_luks_options(
>  static int qemu_rbd_convert_luks_create_options(
>          RbdEncryptionCreateOptionsLUKSBase *luks_opts,
>          rbd_encryption_algorithm_t *alg,
> -        char **passphrase,
> +        const char **passphrase,
>          size_t *passphrase_len,
>          Error **errp)
>  {
> @@ -384,8 +384,7 @@ static int qemu_rbd_encryption_format(rbd_image_t image,
>                                        Error **errp)
>  {
>      int r = 0;
> -    g_autofree char *passphrase = NULL;
> -    size_t passphrase_len;
> +    g_autofree const char *passphrase = NULL;

This looks wierd.  If it is as const string, why are
we free'ing it ?  Either want g_autofree, or const,
but not both.

>      rbd_encryption_format_t format;
>      rbd_encryption_options_t opts;
>      rbd_encryption_luks1_format_options_t luks_opts;
> @@ -407,12 +406,12 @@ static int qemu_rbd_encryption_format(rbd_image_t image,
>              opts_size = sizeof(luks_opts);
>              r = qemu_rbd_convert_luks_create_options(
>                      
> qapi_RbdEncryptionCreateOptionsLUKS_base(&encrypt->u.luks),
> -                    &luks_opts.alg, &passphrase, &passphrase_len, errp);
> +                    &luks_opts.alg, &passphrase, &luks_opts.passphrase_size,
> +                    errp);
>              if (r < 0) {
>                  return r;
>              }
>              luks_opts.passphrase = passphrase;
> -            luks_opts.passphrase_size = passphrase_len;
>              break;
>          }
>          case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2: {
> @@ -423,12 +422,12 @@ static int qemu_rbd_encryption_format(rbd_image_t image,
>              r = qemu_rbd_convert_luks_create_options(
>                      qapi_RbdEncryptionCreateOptionsLUKS2_base(
>                              &encrypt->u.luks2),
> -                    &luks2_opts.alg, &passphrase, &passphrase_len, errp);
> +                    &luks2_opts.alg, &passphrase, 
> &luks2_opts.passphrase_size,
> +                    errp);
>              if (r < 0) {
>                  return r;
>              }
>              luks2_opts.passphrase = passphrase;
> -            luks2_opts.passphrase_size = passphrase_len;
>              break;
>          }
>          default: {
> @@ -466,8 +465,7 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>                                      Error **errp)
>  {
>      int r = 0;
> -    g_autofree char *passphrase = NULL;
> -    size_t passphrase_len;
> +    g_autofree const char *passphrase = NULL;
>      rbd_encryption_luks1_format_options_t luks_opts;
>      rbd_encryption_luks2_format_options_t luks2_opts;
>      rbd_encryption_format_t format;
> @@ -482,12 +480,11 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>              opts_size = sizeof(luks_opts);
>              r = qemu_rbd_convert_luks_options(
>                      qapi_RbdEncryptionOptionsLUKS_base(&encrypt->u.luks),
> -                    &passphrase, &passphrase_len, errp);
> +                    &passphrase, &luks_opts.passphrase_size, errp);
>              if (r < 0) {
>                  return r;
>              }
>              luks_opts.passphrase = passphrase;
> -            luks_opts.passphrase_size = passphrase_len;
>              break;
>          }
>          case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2: {
> @@ -497,12 +494,11 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>              opts_size = sizeof(luks2_opts);
>              r = qemu_rbd_convert_luks_options(
>                      qapi_RbdEncryptionOptionsLUKS2_base(&encrypt->u.luks2),
> -                    &passphrase, &passphrase_len, errp);
> +                    &passphrase, &luks2_opts.passphrase_size, errp);
>              if (r < 0) {
>                  return r;
>              }
>              luks2_opts.passphrase = passphrase;
> -            luks2_opts.passphrase_size = passphrase_len;
>              break;
>          }
>          default: {
> -- 
> 2.25.1
> 
> 

With 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 :|