diff mbox

[3/7] monitor: do_info(): handle new and old info handlers

Message ID 1253136760-3614-4-git-send-email-lcapitulino@redhat.com
State Superseded
Headers show

Commit Message

Luiz Capitulino Sept. 16, 2009, 9:32 p.m. UTC
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(-)

Comments

Markus Armbruster Sept. 23, 2009, 3:46 p.m. UTC | #1
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.

[...]
Luiz Capitulino Sept. 23, 2009, 4:05 p.m. UTC | #2
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.
diff mbox

Patch

diff --git a/monitor.c b/monitor.c
index 17754fb..cfbedf8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -210,6 +210,11 @@  static int monitor_fprintf(FILE *stream, const char *fmt, ...)
     return 0;
 }
 
+static void monitor_print_nothing(Monitor *mon, const QObject *data)
+{
+    return;
+}
+
 static int monitor_handler_ported(const mon_cmd_t *cmd)
 {
     return (cmd->user_print == NULL ? 0 : 1);
@@ -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)
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 400d2bd..b7217f3 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -40,7 +40,7 @@  ETEXI
         .name       = "info",
         .args_type  = "item:s?",
         .handler    = do_info,
-        .user_print = NULL,
+        .user_print = monitor_print_nothing,
         .params     = "[subcommand]",
         .help       = "show various information about the system state"
     },