> -----Original Message----- > From: Qemu-devel > [mailto:[email protected]] On > Behalf Of Max Reitz > Sent: Wednesday, October 14, 2020 5:36 PM > To: Chenqun (kuhn) <[email protected]>; [email protected]; > [email protected] > Cc: [email protected]; ganqixin <[email protected]>; > [email protected]; Zhanghailiang > <[email protected]>; [email protected]; > [email protected]; Li Qiang <[email protected]>; [email protected]; > Laurent Vivier <[email protected]>; [email protected]; Euler Robot > <[email protected]>; [email protected] > 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:[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.
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.
