Re: [Qemu-devel] [PATCH] block: update string sizes for filename, backing_file, exact_filename

[John Snow <jsnow@redhat.com>]

On 01/13/2015 12:03 PM, Jeff Cody wrote:
> The string field entries 'filename', 'backing_file', and
> 'exact_filename' in the BlockDriverState struct are defined as 1024
> bytes.
>
> However, most places that use these values accept a maximum of PATH_MAX
> bytes.  This patch makes the BlockDriverStruct field string sizes match
> the most common usage.
>
> This patch also updates two block drivers that still use 1024-byte sized
> arrays for 'backing_file'.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/mirror.c            | 2 +-
>   block/qapi.c              | 2 +-
>   include/block/block_int.h | 8 ++++----
>   3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 9019d1b..57154eb 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -378,7 +378,7 @@ static void coroutine_fn mirror_run(void *opaque)
>       int64_t sector_num, end, sectors_per_chunk, length;
>       uint64_t last_pause_ns;
>       BlockDriverInfo bdi;
> -    char backing_filename[1024];
> +    char backing_filename[PATH_MAX];
>       int ret = 0;
>       int n;
>
> diff --git a/block/qapi.c b/block/qapi.c
> index a6fd6f7..c097238 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -175,7 +175,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
>   {
>       int64_t size;
>       const char *backing_filename;
> -    char backing_filename2[1024];
> +    char backing_filename2[PATH_MAX];
>       BlockDriverInfo bdi;
>       int ret;
>       Error *err = NULL;
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 06a21dd..e264be9 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -339,13 +339,13 @@ struct BlockDriverState {
>        * regarding this BDS's context */
>       QLIST_HEAD(, BdrvAioNotifier) aio_notifiers;
>
> -    char filename[1024];
> -    char backing_file[1024]; /* if non zero, the image is a diff of
> -                                this file image */
> +    char filename[PATH_MAX];
> +    char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
> +                                    this file image */
>       char backing_format[16]; /* if non-zero and backing_file exists */
>
>       QDict *full_open_options;
> -    char exact_filename[1024];
> +    char exact_filename[PATH_MAX];
>
>       BlockDriverState *backing_hd;
>       BlockDriverState *file;
>

Is it important that qcow2_open seems to enforce a 1023-char length 
backing_file name?

 From qcow2.c, qcow2_open (currently line ~871):
         if (len > MIN(1023, s->cluster_size - 
header.backing_file_offset)) {