Re: [Qemu-devel] [PATCH 3/7] monitor: do_info(): handle new and old info handlers

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 23 Sep 2009 17:46:47 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > do_info() is special, its job is to call 'info handlers'.
> > This is similar to what monitor_handle_command() does,
> > therefore do_info() also has to distinguish among new and
> > old style info handlers.
> >
> > This commit converts do_info() to the new QObject style and
> > makes the appropriate changes so that it can handle both
> > info handlers styles.
> >
> > In the future, when all handlers are converted to QObject's
> > style, it will be possible to share more code with
> > monitor_handle_command().
> >
> > Also note that this commit adds a new function called
> > monitor_print_nothing(), which will be used by converted
> > handlers that don't have data to print in the user protocol.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  monitor.c       |   44 ++++++++++++++++++++++++++++++++------------
> >  qemu-monitor.hx |    2 +-
> >  2 files changed, 33 insertions(+), 13 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 17754fb..cfbedf8 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> [...]
> > @@ -284,24 +289,39 @@ static void do_commit(Monitor *mon, const QDict *qdict)
> >      }
> >  }
> >  
> > -static void do_info(Monitor *mon, const QDict *qdict)
> > +static int do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >  {
> > +    int ret = 0;
> >      const mon_cmd_t *cmd;
> >      const char *item = qdict_get_try_str(qdict, "item");
> > -    void (*handler)(Monitor *);
> >  
> > -    if (!item)
> > -        goto help;
> > -    for(cmd = info_cmds; cmd->name != NULL; cmd++) {
> > +    if (!item) {
> > +        help_cmd(mon, "info");
> > +        return 0;
> > +    }
> > +
> > +    for (cmd = info_cmds; cmd->name != NULL; cmd++) {
> >          if (compare_cmd(item, cmd->name))
> > -            goto found;
> > +            break;
> >      }
> > - help:
> > -    help_cmd(mon, "info");
> > -    return;
> > - found:
> > -    handler = cmd->handler;
> > -    handler(mon);
> > +
> > +    if (cmd->name == NULL)
> > +        return -1;
> > +
> > +    if (monitor_handler_ported(cmd)) {
> > +        int (*handler_new)(Monitor *mon, QObject **ret_data);
> > +
> > +        handler_new = cmd->handler;
> > +        ret = handler_new(mon, ret_data);
> > +
> > +        cmd->user_print(mon, *ret_data);
> > +    } else {
> > +        void (*handler_old)(Monitor *mon);
> > +        handler_old = cmd->handler;
> > +        handler_old(mon);
> > +    }
> > +
> > +    return ret;
> >  }
> >  
> >  static void do_info_version(Monitor *mon)
> 
> Looks like this changes do_info() to fail without printing help when the
> "item" argument is not recognized.  Fine with me, just mention it in the
> commit message, please.

 Actually, it has to maintain compatibility with the current
code, so this is a well spotted bug.

 Will fix, thanks.