Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/5] block: Change bdrv_get_encrypted_filename()

[Wen Congyang]

On 08/06/2015 04:52 AM, Max Reitz wrote:
> Instead of returning a pointer to the filename, copy it into a buffer
> specified by the caller.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block.c               | 24 +++++++++++++++++-------
>  include/block/block.h |  2 +-
>  monitor.c             |  4 +++-
>  3 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/block.c b/block.c
> index d088ee0..e7dd2f1 100644
> --- a/block.c
> +++ b/block.c
> @@ -2760,10 +2760,12 @@ void bdrv_add_key(BlockDriverState *bs, const char 
> *key, Error **errp)
>          }
>      } else {
>          if (bdrv_key_required(bs)) {
> +            char enc_filename[PATH_MAX];

I think it's better to use g_malloc() here.

> +            bdrv_get_encrypted_filename(bs, enc_filename, 
> sizeof(enc_filename));
>              error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
>                        "'%s' (%s) is encrypted",
>                        bdrv_get_device_or_node_name(bs),
> -                      bdrv_get_encrypted_filename(bs));
> +                      enc_filename);
>          }
>      }
>  }
> @@ -2980,14 +2982,22 @@ bool 
> bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs)
>      return false;
>  }
>  
> -const char *bdrv_get_encrypted_filename(BlockDriverState *bs)
> +char *bdrv_get_encrypted_filename(BlockDriverState *bs, char *dest, size_t 
> sz)
>  {
> -    if (bs->backing_hd && bs->backing_hd->encrypted)
> -        return bs->backing_file;
> -    else if (bs->encrypted)
> -        return bs->filename;
> -    else
> +    if (sz > INT_MAX) {
> +        sz = INT_MAX;
> +    }
> +
> +    if (bs->backing_hd && bs->backing_hd->encrypted) {
> +        pstrcpy(dest, sz, bs->backing_file);
> +        return dest;
> +    } else if (bs->encrypted) {
> +        pstrcpy(dest, sz, bs->filename);
> +        return dest;
> +    } else {
> +        dest[0] = '\0';
>          return NULL;
> +    }
>  }
>  
>  void bdrv_get_backing_filename(BlockDriverState *bs,
> diff --git a/include/block/block.h b/include/block/block.h
> index 37916f7..a78e4f1 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -430,7 +430,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
>                              int64_t *cluster_sector_num,
>                              int *cluster_nb_sectors);
>  
> -const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
> +char *bdrv_get_encrypted_filename(BlockDriverState *bs, char *dest, size_t 
> sz);
>  void bdrv_get_backing_filename(BlockDriverState *bs,
>                                 char *filename, int filename_size);
>  void bdrv_get_full_backing_filename(BlockDriverState *bs,
> diff --git a/monitor.c b/monitor.c
> index aeea2b5..cfdf781 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -5292,10 +5292,12 @@ int monitor_read_bdrv_key_start(Monitor *mon, 
> BlockDriverState *bs,
>                                  BlockCompletionFunc *completion_cb,
>                                  void *opaque)
>  {
> +    char enc_filename[PATH_MAX];

same too.

Thanks
Wen Congyang

>      int err;
>  
> +    bdrv_get_encrypted_filename(bs, enc_filename, sizeof(enc_filename));
>      monitor_printf(mon, "%s (%s) is encrypted.\n", bdrv_get_device_name(bs),
> -                   bdrv_get_encrypted_filename(bs));
> +                   enc_filename);
>  
>      mon->password_completion_cb = completion_cb;
>      mon->password_opaque = opaque;
>