Re: [Qemu-devel] [PATCH V10 00/15] monitor: support sub command group in auto completion and help

[Luiz Capitulino]

On Fri, 30 Aug 2013 10:22:43 +0800
Wenchao Xia <address@hidden> wrote:

> 于 2013-8-29 22:16, Luiz Capitulino 写道:
> > On Tue, 27 Aug 2013 20:38:12 +0800
> > Wenchao Xia <address@hidden> wrote:
> >
> >> This series make auto completion and help functions works normal for sub
> >> command, by using reentrant functions. In order to do that, global 
> >> variables
> >> are not directly used in those functions any more. With this series, 
> >> cmd_table
> >> is a member of structure Monitor so it is possible to create a monitor with
> >> different command table now, auto completion will work in that monitor. In
> >> short, "info" is not treated as a special case now, this series ensure help
> >> and auto complete function works normal for any sub command added in the 
> >> future.
> >>
> >> Patch 5 replaced cur_mon with rs->mon, it is safe because:
> >> monitor_init() calls readline_init() which initialize mon->rs, result is
> >> mon->rs->mon == mon. Then qemu_chr_add_handlers() is called, which make
> >> monitor_read() function take *mon as its opaque. Later, when user input,
> >> monitor_read() is called, where cur_mon is set to *mon by "cur_mon = 
> >> opaque".
> >> If qemu's monitors run in one thread, then later in readline_handle_byte()
> >> and readline_comletion(), cur_mon is actually equal to rs->mon, in another
> >> word, it points to the monitor instance, so it is safe to replace *cur_mon
> >> in those functions.
> >>
> >> Thanks for Luiz and Eric for reviewing.
> >
> > Applied to the qmp branch, thanks.
> >
> > Btw, I'm assuming that the patches that contain Eric's reviewed-by didn't
> > change since he reviewed them. Personally, I don't think it's right to carry
> > reviewed-bys when the patch changes, even if it's something small or
> > obvious.
> >
>    I remember that the "Reviewed-by" were removed on all changed
> patches, except:
>    Patches of V7 have typo issues which were pointed
> out by Eric with "Reviewed-by", for those I fixed the typo and kept the
> "Reviewed by".
>    Some patches were not modified but the line number may change,
> since previous patch may have inserted new lines. For those I didn't
> remove "Reviewed-by".
>    If the above is not appropriate, I'll avoid it.

Looks fine to me.