qemu-trivial
[Top][All Lists]
Advanced

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

RE: [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable


From: Chenqun (kuhn)
Subject: RE: [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable warning
Date: Wed, 14 Oct 2020 11:21:15 +0000

> -----Original Message-----
> From: Qemu-devel
> [mailto:qemu-devel-bounces+kuhn.chenqun=huawei.com@nongnu.org] On
> Behalf Of Max Reitz
> Sent: Wednesday, October 14, 2020 5:36 PM
> To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; qemu-devel@nongnu.org;
> qemu-trivial@nongnu.org
> Cc: fam@euphon.net; ganqixin <ganqixin@huawei.com>;
> vsementsov@virtuozzo.com; Zhanghailiang
> <zhang.zhanghailiang@huawei.com>; qemu-block@nongnu.org;
> quintela@redhat.com; Li Qiang <liq3ea@gmail.com>; dgilbert@redhat.com;
> Laurent Vivier <laurent@vivier.eu>; stefanha@redhat.com; Euler Robot
> <euler.robot@huawei.com>; jsnow@redhat.com
> Subject: Re: [PATCH v2] migration/block-dirty-bitmap: fix uninitialized 
> variable
> warning
> 
> On 14.10.20 03:03, Chenqun (kuhn) wrote:
> >
> >
> >> -----Original Message-----
> >> From: Max Reitz [mailto:mreitz@redhat.com]
> >> Sent: Tuesday, October 13, 2020 10:47 PM
> >> To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>;
> qemu-devel@nongnu.org;
> >> qemu-trivial@nongnu.org
> >> Cc: vsementsov@virtuozzo.com; stefanha@redhat.com; fam@euphon.net;
> >> eblake@redhat.com; jsnow@redhat.com; quintela@redhat.com;
> >> dgilbert@redhat.com; Zhanghailiang <zhang.zhanghailiang@huawei.com>;
> >> ganqixin <ganqixin@huawei.com>; qemu-block@nongnu.org; Euler Robot
> >> <euler.robot@huawei.com>; Laurent Vivier <laurent@vivier.eu>; Li
> >> Qiang <liq3ea@gmail.com>
> >> 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 <euler.robot@huawei.com>
> >>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> >>> ---
> >>> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >>> Cc: Laurent Vivier <laurent@vivier.eu>
> >>> Cc: Li Qiang <liq3ea@gmail.com>
> >>> ---
> >>>  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.  

I think my description is not accurate. Forgive me for being a non-native 
English speaker.
The variable 'bitmap_name' assignment maybe failed at the beginning of the 
block, because reading the 's->bitmap_alias' maybe failed.

>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.
>
I see what you mean. Separate variable declarations from variable assignments.
You're right!  I will update it later.

Thanks,
Chen Qun
> (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.


reply via email to

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