Re: [Qemu-block] [PATCH v3 07/25] block: bdrv_get_full_backing_filename's ret. val.

[Alberto Garcia]

On Fri 16 Dec 2016 02:26:29 PM CET, Max Reitz wrote:

>>>  int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
>>>                             const char *bdref_key, Error **errp)
>>>  {
>>> -    char *backing_filename = g_malloc0(PATH_MAX);
>>> +    char *backing_filename = NULL;
>>>      char *bdref_key_dot;
>>>      const char *reference = NULL;
>>>      int ret = 0;
>>> @@ -1511,7 +1505,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, 
>>> QDict *parent_options,
>>>  
>>>      reference = qdict_get_try_str(parent_options, bdref_key);
>>>      if (reference || qdict_haskey(options, "file.filename")) {
>>> -        backing_filename[0] = '\0';
>>> +        backing_filename = NULL;
>> 
>> You're making it NULL, but it's NULL already.
>
> Yes, so I hope the compiler can optimize it away. :-)
>
> I initialized it to NULL here to make it more clear that under the
> given condition we do not have a backing filename (so that it's clear
> from a single look at this if-else block what exactly happens in each
> case without having to know the state of variables beforehand).

Ok, I suspected that. Even if we keep it, I think it wouldn't be bad to
leave a comment saying that it's there on purpose and it's not being set
twice accidentally.

Otherwise whoever reads that code in the future might think that it's a
sign of a merge that wasn't carefully reviewed.

But I won't oppose if you prefer to leave it like that.

Reviewed-by: Alberto Garcia <address@hidden>

Berto