Luiz Capitulino <lcapitul...@redhat.com> 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 <lcapitul...@redhat.com>
> ---
> 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)
