Re: [Qemu-block] [PATCH v6 09/25] block: Fix bdrv_find_backing_image()

[Alberto Garcia]

On Fri 29 Sep 2017 06:53:31 PM CEST, Max Reitz wrote:
> @@ -4096,22 +4086,31 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>          } else {
>              /* If not an absolute filename path, make it relative to the 
> current
>               * image's filename path */
> -            path_combine_deprecated(filename_tmp, PATH_MAX, 
> curr_bs->filename,
> -                                    backing_file);
> +            filename_tmp = bdrv_make_absolute_filename(curr_bs, backing_file,
> +                                                       NULL);
> +            if (!filename_tmp) {
> +                continue;
> +            }
>  
>              /* We are going to compare absolute pathnames */
>              if (!realpath(filename_tmp, filename_full)) {
> +                g_free(filename_tmp);
>                  continue;
>              }
> +            g_free(filename_tmp);

This feels a bit too verbose, doesn't it? (especially because you're
doing the same thing twice, see below). It could be made a bit shorter,
something like:

    bool have_filename = filename_tmp && realpath(filename_tmp, filename_full);
    g_free(filename_tmp);
    if (!have_filename) {
         continue;
    }

> -            path_combine_deprecated(filename_tmp, PATH_MAX, 
> curr_bs->filename,
> -                                    curr_bs->backing_file);
> +            filename_tmp = bdrv_get_full_backing_filename(curr_bs, NULL);
> +            if (!filename_tmp) {
> +                continue;
> +            }
>  
>              if (!realpath(filename_tmp, backing_file_full)) {
> +                g_free(filename_tmp);
>                  continue;
>              }
> +            g_free(filename_tmp);

Berto