Eric Blake <ebl...@redhat.com> writes:
> On 05/22/2015 05:36 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <arm...@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 <ebl...@redhat.com>
Thanks!
