[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 07/25] block: bdrv_get_full_backing_filename'
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v3 07/25] block: bdrv_get_full_backing_filename's ret. val. |
Date: |
Fri, 16 Dec 2016 14:26:29 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
On 15.12.2016 17:39, Alberto Garcia wrote:
> On Wed 30 Nov 2016 02:18:33 AM 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).
So it was intentional, but I can either change it so that
backing_filename would no longer be initialized to NULL (meaning the
first "goto free_exit;" would need to be just a "return 0;") or make it
a comment (/* backing_filename is already NULL */). I'm leaning towards
the latter because it would mean less changes to this series, although I
personally think that simply explicitly setting backing_filename to NULL
instead of such a comment is just as good.
So if I have to send a v4 (which I probably will), I'll make it a
comment, but I probably won't send a v4 just for this (I'm sure there
are other good reasons, however :-)).
And thanks for reviewing!
Max
signature.asc
Description: OpenPGP digital signature