[Libguestfs] [libguestfs-common PATCH 1/2] options/keys: key_store_import_key(): un-constify "key" parameter

[Richard W.M. Jones]

On Mon, May 15, 2023 at 07:49:23PM +0200, Laszlo Ersek wrote:
> The key_store_import_key() function is called from both C-language
> utilities -- via key_store_add_from_selector() -- and OCaml-language ones
> -- via guestfs_int_mllib_inspect_decrypt().  We currently declare the
> function's second parameter as
> 
>   const struct key_store_key *key
> 
> That is however a superficial, if not outright false, promise: in
> key_store_import_key(), we take ownership of all three string fields
> within "key", as evidenced by free_key_store(), where we free() all three
> strings. With the same effort, we might as well modify the contents of
> those strings at once. Drop "const". (None of the callers care, but let's
> be honest.)

I'm not completely certain what the rules are here.  Can't you have a
const struct containing non-const strings?  However I agree it looks
confusing so that's a decent reason to change it, so:

Reviewed-by: Richard W.M. Jones <rjones at redhat.com>

> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2168506
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  options/options.h | 3 ++-
>  options/keys.c    | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/options/options.h b/options/options.h
> index 94573ee063bb..94e8b9eef43e 100644
> --- a/options/options.h
> +++ b/options/options.h
> @@ -169,7 +169,8 @@ extern struct matching_key *get_keys (struct key_store *ks, const char *device,
>                                        const char *uuid, size_t *nr_matches);
>  extern void free_keys (struct matching_key *keys, size_t nr_matches);
>  extern struct key_store *key_store_add_from_selector (struct key_store *ks, const char *selector);
> -extern struct key_store *key_store_import_key (struct key_store *ks, const struct key_store_key *key);
> +extern struct key_store *key_store_import_key (struct key_store *ks,
> +                                               struct key_store_key *key);
>  extern bool key_store_requires_network (const struct key_store *ks);
>  extern void free_key_store (struct key_store *ks);
>  
> diff --git a/options/keys.c b/options/keys.c
> index 48f1bc7c7c47..bc7459749276 100644
> --- a/options/keys.c
> +++ b/options/keys.c
> @@ -261,7 +261,7 @@ key_store_add_from_selector (struct key_store *ks, const char *selector)
>  }
>  
>  struct key_store *
> -key_store_import_key (struct key_store *ks, const struct key_store_key *key)
> +key_store_import_key (struct key_store *ks, struct key_store_key *key)
>  {
>    struct key_store_key *new_keys;

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v