Message ID | 1432294585-5984-3-git-send-email-armbru@redhat.com |
---|---|
State | New |
Headers | show |
On 05/22/2015 05:36 AM, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- 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. > > 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 <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 05/22/2015 05:36 AM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- > > 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 <eblake@redhat.com> Thanks!
diff --git a/monitor.c b/monitor.c index 9a0c3b7..5330e61 100644 --- a/monitor.c +++ b/monitor.c @@ -4045,18 +4045,6 @@ void monitor_set_error(Monitor *mon, QError *qerror) } } -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); - } -} - 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); -} - 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); + } + } err_out: - monitor_protocol_emitter(mon, NULL); -out: + monitor_protocol_emitter(mon, data); + qobject_decref(data); QDECREF(input); QDECREF(args); }
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- monitor.c | 40 +++++++++++----------------------------- 1 file changed, 11 insertions(+), 29 deletions(-)