qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 10/13] block: Simplify bdrv_append_temp_snaps


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v3 10/13] block: Simplify bdrv_append_temp_snapshot() logic
Date: Thu, 6 Apr 2017 07:52:05 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

On 04/06/2017 04:00 AM, Kevin Wolf wrote:
> Am 05.04.2017 um 21:47 hat Eric Blake geschrieben:
>> Noticed while checking Coccinelle results. Naming a label 'out:'
>> when it is only used on error paths is weird; meanwhile we know
>> that snapshot_options is NULL on success and that QDECREF(NULL)
>> is safe.  So merge the two exit paths into one.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> ---
>>  block.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 9b87bf6..a13625f 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2209,13 +2209,10 @@ static BlockDriverState 
>> *bdrv_append_temp_snapshot(BlockDriverState *bs,
>>          goto out;
>>      }
>>
>> +out:
>> +    QDECREF(snapshot_options);
>>      g_free(tmp_filename);
>>      return bs_snapshot;
> 
> bs_snapshot is uninitialised or at least the wrong return value in error
> cases. (Hm... Shouldn't the compiler catch the uninitialised part?)

Odd, and I agree that I recall the compiler generally able to catch that
(maybe it's a matter of compiling with -g vs. -O2).  I'm surprised the
autobuilder didn't flag it.  (I think I missed it due to rebase churn on
my end).  The obvious fix is to:

-    BlockDriverState *bs_snapshot;
+    BlockDriverState *bs_snapshot = NULL;

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]