Re: [Qemu-devel] [PATCH 2/3] QMP: Drop dead code

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> The first if/else clause in handler_audit() makes no sense for two
> reasons:
>
>   1. this function is now called only by QMP code, so testing if
>      it's a QMP call makes no sense anymore
>
>   2. the else clause first asserts that there's no error in the
>      monitor object, then it tries to free it!
>
> Just drop it.
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> ---
>  monitor.c |   74 
> ++++++++++++++++++++++++++++---------------------------------
>  1 files changed, 34 insertions(+), 40 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index c4efe58..30be273 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3853,49 +3853,43 @@ void monitor_set_error(Monitor *mon, QError *qerror)
>  
>  static void handler_audit(Monitor *mon, const mon_cmd_t *cmd, int ret)
>  {
> -    if (monitor_ctrl_mode(mon)) {
> -        if (ret && !monitor_has_error(mon)) {
> -            /*
> -             * If it returns failure, it must have passed on error.
> -             *
> -             * Action: Report an internal error to the client if in QMP.
> -             */
> -            qerror_report(QERR_UNDEFINED_ERROR);
> -            MON_DEBUG("command '%s' returned failure but did not pass an 
> error\n",
> -                      cmd->name);
> -        }
> +    if (ret && !monitor_has_error(mon)) {
> +        /*
> +         * If it returns failure, it must have passed on error.
> +         *
> +         * Action: Report an internal error to the client if in QMP.
> +         */
> +        qerror_report(QERR_UNDEFINED_ERROR);
> +        MON_DEBUG("command '%s' returned failure but did not pass an 
> error\n",
> +                  cmd->name);
> +    }
>  
>  #ifdef CONFIG_DEBUG_MONITOR
> -        if (!ret && monitor_has_error(mon)) {
> -            /*
> -             * If it returns success, it must not have passed an error.
> -             *
> -             * Action: Report the passed error to the client.
> -             */
> -            MON_DEBUG("command '%s' returned success but passed an error\n",
> -                      cmd->name);
> -        }
> -
> -        if (mon_print_count_get(mon) > 0 && strcmp(cmd->name, "info") != 0) {
> -            /*
> -             * Handlers should not call Monitor print functions.
> -             *
> -             * Action: Ignore them in QMP.
> -             *
> -             * (XXX: we don't check any 'info' or 'query' command here
> -             * because the user print function _is_ called by do_info(), 
> hence
> -             * we will trigger this check. This problem will go away when we
> -             * make 'query' commands real and kill do_info())
> -             */
> -            MON_DEBUG("command '%s' called print functions %d time(s)\n",
> -                      cmd->name, mon_print_count_get(mon));
> -        }
> -#endif
> -    } else {
> -        assert(!monitor_has_error(mon));
> -        QDECREF(mon->error);
> -        mon->error = NULL;
> +    if (!ret && monitor_has_error(mon)) {
> +        /*
> +         * If it returns success, it must not have passed an error.
> +         *
> +         * Action: Report the passed error to the client.
> +         */
> +        MON_DEBUG("command '%s' returned success but passed an error\n",
> +                  cmd->name);
> +    }
> +
> +    if (mon_print_count_get(mon) > 0 && strcmp(cmd->name, "info") != 0) {
> +        /*
> +         * Handlers should not call Monitor print functions.
> +         *
> +         * Action: Ignore them in QMP.
> +         *
> +         * (XXX: we don't check any 'info' or 'query' command here
> +         * because the user print function _is_ called by do_info(), hence
> +         * we will trigger this check. This problem will go away when we
> +         * make 'query' commands real and kill do_info())
> +         */
> +        MON_DEBUG("command '%s' called print functions %d time(s)\n",
> +                  cmd->name, mon_print_count_get(mon));
>      }
> +#endif
>  }
>  
>  static void handle_user_command(Monitor *mon, const char *cmdline)

For easier review:

$ git-diff -w
diff --git a/monitor.c b/monitor.c
index 8cee35d..138db06 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3853,7 +3853,6 @@ void monitor_set_error(Monitor *mon, QError *qerror)
 
 static void handler_audit(Monitor *mon, const mon_cmd_t *cmd, int ret)
 {
-    if (monitor_ctrl_mode(mon)) {
         if (ret && !monitor_has_error(mon)) {
             /*
              * If it returns failure, it must have passed on error.
@@ -3891,11 +3890,6 @@ static void handler_audit(Monitor *mon, const mon_cmd_t 
*cmd, int ret)
                       cmd->name, mon_print_count_get(mon));
         }
 #endif
-    } else {
-        assert(!monitor_has_error(mon));
-        QDECREF(mon->error);
-        mon->error = NULL;
-    }
 }
 
 static void handle_user_command(Monitor *mon, const char *cmdline)