* [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable warning
@ 2020-10-13 12:33 Chen Qun
2020-10-13 14:47 ` Max Reitz
0 siblings, 1 reply; 5+ messages in thread
From: Chen Qun @ 2020-10-13 12:33 UTC (permalink / raw)
To: qemu-devel, qemu-trivial
Cc: fam, ganqixin, vsementsov, zhang.zhanghailiang, qemu-block,
quintela, Li Qiang, dgilbert, mreitz, stefanha, Euler Robot,
Chen Qun, jsnow, Laurent Vivier
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(-)
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;
+ if (!s->cancelled && bitmap_alias_map) {
bitmap_name = g_hash_table_lookup(bitmap_alias_map,
s->bitmap_alias);
if (!bitmap_name) {
@@ -1081,9 +1079,6 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
s->bs->node_name, s->node_alias);
cancel_incoming_locked(s);
}
- } else {
- bitmap_name = s->bitmap_alias;
- }
}
if (!s->cancelled) {
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable warning
2020-10-13 12:33 [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable warning Chen Qun
@ 2020-10-13 14:47 ` Max Reitz
2020-10-14 1:03 ` Chenqun (kuhn)
0 siblings, 1 reply; 5+ messages in thread
From: Max Reitz @ 2020-10-13 14:47 UTC (permalink / raw)
To: Chen Qun, qemu-devel, qemu-trivial
Cc: fam, ganqixin, vsementsov, zhang.zhanghailiang, qemu-block,
quintela, Li Qiang, dgilbert, Laurent Vivier, stefanha,
Euler Robot, jsnow
[-- Attachment #1.1: Type: text/plain, Size: 2514 bytes --]
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.)
> + if (!s->cancelled && bitmap_alias_map) {
> bitmap_name = g_hash_table_lookup(bitmap_alias_map,
> s->bitmap_alias);
This block of course needs to be re-indented.
Max
> if (!bitmap_name) {
> @@ -1081,9 +1079,6 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
> s->bs->node_name, s->node_alias);
> cancel_incoming_locked(s);
> }
> - } else {
> - bitmap_name = s->bitmap_alias;
> - }
> }
>
> if (!s->cancelled) {
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable warning
2020-10-13 14:47 ` Max Reitz
@ 2020-10-14 1:03 ` Chenqun (kuhn)
2020-10-14 9:35 ` Max Reitz
0 siblings, 1 reply; 5+ messages in thread
From: Chenqun (kuhn) @ 2020-10-14 1:03 UTC (permalink / raw)
To: Max Reitz, qemu-devel@nongnu.org, qemu-trivial@nongnu.org
Cc: fam@euphon.net, ganqixin, vsementsov@virtuozzo.com, Zhanghailiang,
qemu-block@nongnu.org, quintela@redhat.com, Li Qiang,
dgilbert@redhat.com, Laurent Vivier, stefanha@redhat.com,
Euler Robot, jsnow@redhat.com
> -----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?
> > + if (!s->cancelled && bitmap_alias_map) {
> > bitmap_name =
> g_hash_table_lookup(bitmap_alias_map,
> >
> s->bitmap_alias);
>
> This block of course needs to be re-indented.
>
I forgot this. I will update it later.
Thanks,
ChenQun
>
> > if (!bitmap_name) {
> > @@ -1081,9 +1079,6 @@ static int dirty_bitmap_load_header(QEMUFile *f,
> DBMLoadState *s,
> > s->bs->node_name,
> s->node_alias);
> > cancel_incoming_locked(s);
> > }
> > - } else {
> > - bitmap_name = s->bitmap_alias;
> > - }
> > }
> >
> > if (!s->cancelled) {
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable warning
2020-10-14 1:03 ` Chenqun (kuhn)
@ 2020-10-14 9:35 ` Max Reitz
2020-10-14 11:21 ` Chenqun (kuhn)
0 siblings, 1 reply; 5+ messages in thread
From: Max Reitz @ 2020-10-14 9:35 UTC (permalink / raw)
To: Chenqun (kuhn), qemu-devel@nongnu.org, qemu-trivial@nongnu.org
Cc: fam@euphon.net, ganqixin, vsementsov@virtuozzo.com, Zhanghailiang,
qemu-block@nongnu.org, quintela@redhat.com, Li Qiang,
dgilbert@redhat.com, Laurent Vivier, stefanha@redhat.com,
Euler Robot, jsnow@redhat.com
[-- Attachment #1.1: Type: text/plain, Size: 3560 bytes --]
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. 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable warning
2020-10-14 9:35 ` Max Reitz
@ 2020-10-14 11:21 ` Chenqun (kuhn)
0 siblings, 0 replies; 5+ messages in thread
From: Chenqun (kuhn) @ 2020-10-14 11:21 UTC (permalink / raw)
To: Max Reitz, qemu-devel@nongnu.org, qemu-trivial@nongnu.org
Cc: fam@euphon.net, vsementsov@virtuozzo.com, Zhanghailiang,
qemu-block@nongnu.org, quintela@redhat.com, jsnow@redhat.com,
Li Qiang, dgilbert@redhat.com, Laurent Vivier, ganqixin,
Euler Robot, stefanha@redhat.com
> -----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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-14 11:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-13 12:33 [PATCH v2] migration/block-dirty-bitmap: fix uninitialized variable warning Chen Qun
2020-10-13 14:47 ` Max Reitz
2020-10-14 1:03 ` Chenqun (kuhn)
2020-10-14 9:35 ` Max Reitz
2020-10-14 11:21 ` Chenqun (kuhn)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).