From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47574) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxAjf-00084O-18 for qemu-devel@nongnu.org; Tue, 26 May 2015 05:01:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YxAjc-0007ex-7B for qemu-devel@nongnu.org; Tue, 26 May 2015 05:01:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48603) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxAjb-0007eZ-VX for qemu-devel@nongnu.org; Tue, 26 May 2015 05:01:16 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t4Q91E4H030875 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 26 May 2015 05:01:14 -0400 From: Markus Armbruster References: <1432294585-5984-1-git-send-email-armbru@redhat.com> <1432294585-5984-3-git-send-email-armbru@redhat.com> <555F9D68.8010102@redhat.com> Date: Tue, 26 May 2015 11:01:11 +0200 In-Reply-To: <555F9D68.8010102@redhat.com> (Eric Blake's message of "Fri, 22 May 2015 15:19:36 -0600") Message-ID: <87vbffemqw.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 02/20] monitor: Clean up after previous commit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, lcapitulino@redhat.com Eric Blake writes: > On 05/22/2015 05:36 AM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster >> --- > > Commit message is sparse; I would have mentioned [1] and [2]. > >> monitor.c | 40 +++++++++++----------------------------- >> 1 file changed, 11 insertions(+), 29 deletions(-) >> >> >> -static void handler_audit(Monitor *mon, const mon_cmd_t *cmd, int ret) >> -{ >> - 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); >> - } >> -} > > [2] - handler_audit() goes away now that it has no caller > >> - >> static void handle_user_command(Monitor *mon, const char *cmdline) >> { >> QDict *qdict; >> @@ -5015,28 +5003,17 @@ static QDict *qmp_check_input_obj(QObject *input_obj) >> return input_dict; >> } >> >> -static void qmp_call_cmd(Monitor *mon, const mon_cmd_t *cmd, >> - const QDict *params) >> -{ >> - int ret; >> - QObject *data = NULL; >> - >> - ret = cmd->mhandler.cmd_new(mon, params, &data); >> - handler_audit(mon, cmd, ret); >> - monitor_protocol_emitter(mon, data); >> - qobject_decref(data); >> -} > > [1] - qmp_call_cmd() is inlined and simplified... > >> - >> static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) >> { >> int err; >> - QObject *obj; >> + QObject *obj, *data; >> QDict *input, *args; >> const mon_cmd_t *cmd; >> const char *cmd_name; >> Monitor *mon = cur_mon; >> >> args = input = NULL; >> + data = NULL; >> >> obj = json_parser_parse(tokens, NULL); >> if (!obj) { >> @@ -5079,12 +5056,17 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) >> goto err_out; >> } >> >> - qmp_call_cmd(mon, cmd, args); >> - goto out; >> + if (cmd->mhandler.cmd_new(mon, args, &data)) { >> + /* Command failed... */ >> + if (!monitor_has_error(mon)) { >> + /* ... without setting an error, so make one up */ >> + qerror_report(QERR_UNDEFINED_ERROR); >> + } >> + } > > ...into its lone caller. Can do. >> >> err_out: >> - monitor_protocol_emitter(mon, NULL); >> -out: >> + monitor_protocol_emitter(mon, data); >> + qobject_decref(data); >> QDECREF(input); >> QDECREF(args); >> } >> > > But the code cleanup itself looks sane, so with the improved commit message: > > Reviewed-by: Eric Blake Thanks!