On 14.10.20 03:03, Chenqun (kuhn) wrote:
> 
> 
>> -----Original Message-----
>> From: Max Reitz [mailto:[email protected]]
>> Sent: Tuesday, October 13, 2020 10:47 PM
>> To: Chenqun (kuhn) <[email protected]>; [email protected];
>> [email protected]
>> Cc: [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; Zhanghailiang <[email protected]>;
>> ganqixin <[email protected]>; [email protected]; Euler Robot
>> <[email protected]>; Laurent Vivier <[email protected]>; Li Qiang
>> <[email protected]>
>> Subject: Re: [PATCH v2] migration/block-dirty-bitmap: fix uninitialized 
>> variable
>> warning
>>
>> On 13.10.20 14:33, Chen Qun wrote:
>>> A default value is provided for the variable 'bitmap_name' to avoid compiler
>> warning.
>>>
>>> The compiler show warning:
>>> migration/block-dirty-bitmap.c:1090:13: warning: ‘bitmap_name’
>>> may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>        g_strlcpy(s->bitmap_name, bitmap_name,
>> sizeof(s->bitmap_name));
>>>
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Reported-by: Euler Robot <[email protected]>
>>> Signed-off-by: Chen Qun <[email protected]>
>>> ---
>>> Cc: Vladimir Sementsov-Ogievskiy <[email protected]>
>>> Cc: Laurent Vivier <[email protected]>
>>> Cc: Li Qiang <[email protected]>
>>> ---
>>>  migration/block-dirty-bitmap.c | 9 ++-------
>>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> No objections, semantically, but...
>>
>>> diff --git a/migration/block-dirty-bitmap.c
>>> b/migration/block-dirty-bitmap.c index 5bef793ac0..bcb79c04ce 100644
>>> --- a/migration/block-dirty-bitmap.c
>>> +++ b/migration/block-dirty-bitmap.c
>>> @@ -1064,15 +1064,13 @@ static int dirty_bitmap_load_header(QEMUFile
>> *f, DBMLoadState *s,
>>>      assert(nothing || s->cancelled || !!alias_map ==
>>> !!bitmap_alias_map);
>>>
>>>      if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME) {
>>> -        const char *bitmap_name;
>>> -
>>>          if (!qemu_get_counted_string(f, s->bitmap_alias)) {
>>>              error_report("Unable to read bitmap alias string");
>>>              return -EINVAL;
>>>          }
>>>
>>> -        if (!s->cancelled) {
>>> -            if (bitmap_alias_map) {
>>> +        const char *bitmap_name = s->bitmap_alias;
>>
>> qemu’s coding style mandates declarations to be placed at the beginning of
>> their block, so the declaration has to stay where it is.  (Putting the 
>> assignment
>> here looks reasonable.)
>>
> Hi Max,
>   Declaration variables here to ensure that the above exceptions(Unable to 
> read bitmap alias string) are avoided.
> If the declaration has to stay where it is, is there a possibility that the 
> assignment fails?

I don’t understand what you mean.  A declaration without initialization
isn’t and doesn’t contain an expression, it isn’t even a statement, so
it has no side effects.[1]

Placing the declaration (without an initialization) at the top of the
block makes no semantic difference.

(As I said, I’d keep the assignment “bitmap_name = s->bitmap_alias”
where you put it.  I think it would technically actually be correct to
put it into the declaration at the start of the block as an initializer,
but that would look weird.)

Max

[1] I suppose exceptions apply for types with constructors, but those
don’t exist in plain C.

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to