Re: [Qemu-devel] [PATCH V3 1/7] monitor: discard global variable *cur_mon in completion functions

[Wenchao Xia]

于 2013-6-28 5:26, Eric Blake 写道:
> On 06/26/2013 09:27 PM, Wenchao Xia wrote:
> > Parameter *mon is added to replace *cur_mon, and readline_completion()
> > pass rs->mon as value, which should be initialized in readline_init()
> > called by monitor_init(). In short, structure ReadLineState controls
> > where the action would be taken now.
> >
> > Signed-off-by: Wenchao Xia <address@hidden>
> > ---
>
> > @@ -4075,20 +4075,27 @@ static void file_completion(const char *input)
> >               if (stat(file, &sb) == 0 && S_ISDIR(sb.st_mode)) {
> >                   pstrcat(file, sizeof(file), "/");
> >               }
> > -            readline_add_completion(cur_mon->rs, file);
> > +            readline_add_completion(mon->rs, file);
> >           }
> >       }
> >       closedir(ffs);
>
> Oops, I started reviewing that before noticing you posted a v3.  At any
> rate, my review of v2 still stands on this patch.  Meanwhile, I just
> barely noticed that this pre-existing code might cause a -Wshadow
> warning in the future if <strings.h> is ever included.  I'm not sure
> what qemu's policy is on being clean against -Wshadow warnings, but I
> personally tend to avoid local variables whose name might conflict with
> global functions.
  Thanks for reviewing. I'll fix what you mentioned in v2.

  For the -Wshadow warning, I guess you mean "file" right? It would
be a bit hard to check this warning for new patch, since this warning
now exist in many code. In my opinion, it would be better to have a
separate series to clean up this warnings first, then check new patches
for this issue.

--
Best Regards

Wenchao Xia