> >>>> Le 10/10/2020 à 13:07, Chen Qun a écrit :
> >>>>> This if statement judgment is redundant and it will cause a 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.ro...@huawei.com>
> >>>>> Signed-off-by: Chen Qun <kuhn.chen...@huawei.com>
> >>>>> ---
> >>>>> migration/block-dirty-bitmap.c | 2 --
> >>>>> 1 file changed, 2 deletions(-)
> >>>>>
> >>>>> diff --git a/migration/block-dirty-bitmap.c
> >>>>> b/migration/block-dirty-bitmap.c index 5bef793ac0..e09ea4f22b
> >>>>> 100644
> >>>>> --- a/migration/block-dirty-bitmap.c
> >>>>> +++ b/migration/block-dirty-bitmap.c
> >>>>> @@ -1084,9 +1084,7 @@ static int
> dirty_bitmap_load_header(QEMUFile
> >> *f, DBMLoadState *s,
> >>>>> } else {
> >>>>> bitmap_name = s->bitmap_alias;
> >>>>> }
> >>>>> - }
> >>>>>
> >>>>> - if (!s->cancelled) {
> >>>>> g_strlcpy(s->bitmap_name, bitmap_name,
> >> sizeof(s->bitmap_name));
> >>>>> s->bitmap = bdrv_find_dirty_bitmap(s->bs,
> >>>>> s->bitmap_name);
> >>>>>
> >>>>>
> >>>>
> >>>> I don't think it's correct as "cancel_incoming_locked(s)" can
> >>>> change the value of "s->cancelled".
> >>>>
> >>>
> >>> Hi Laurent,
> >>>
> >>> You're right. So I think this can simply assign 'bitmap_name' to
> >>> NULL to make compiler happy.
> >>
> >> Yes, and adding a comment before the second "if (!s->cancelled) {" to
> >> explain the value can be changed by "cancel_incoming_locked(s)" would
> >> avoid to have this kind of patch posted regularly to the ML.
> >>
> > Hi Laurent,
> >
> > We give the bitmap_name a default value (s->bitmap_alias) so that we can
> remove the assignment of the else branch statement.
> >
> > And then we merge the two if statements("if (!s->cancelled)", "if
> (bitmap_alias_map)") , avoids confusion the two "if (!s->cancelled)".
> >
> > Thanks,
> > Chen Qun
> >
> >
> > The code show as that:
> > @@ -1064,15 +1064,14 @@ 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;
> > + const char *bitmap_name = s->bitmap_alias;
> >
> > 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) {
> > + if (!s->cancelled && bitmap_alias_map) {
> > bitmap_name =
> g_hash_table_lookup(bitmap_alias_map,
> >
> s->bitmap_alias);
> > if (!bitmap_name) {
> > @@ -1081,9 +1080,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) {
> >
>
> Sounds good.
>
OK, I'll update it later.
Thanks,
Chen Qun
