[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/20] monitor: Clean up after previous commit
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 02/20] monitor: Clean up after previous commit |
Date: |
Fri, 22 May 2015 15:19:36 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 05/22/2015 05:36 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
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 <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH 03/20] monitor: Improve and document client_migrate_info protocol error, (continued)
- [Qemu-devel] [PATCH 08/20] monitor: Drop unused "new" HMP command interface, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 10/20] monitor: Propagate errors through qmp_check_input_obj(), Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 12/20] monitor: Inline monitor_has_error() into its only caller, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 04/20] monitor: Convert client_migrate_info to QAPI, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 02/20] monitor: Clean up after previous commit, Markus Armbruster, 2015/05/22
- Re: [Qemu-devel] [PATCH 02/20] monitor: Clean up after previous commit,
Eric Blake <=
- [Qemu-devel] [PATCH 17/20] monitor: Drop do_qmp_capabilities()'s superfluous QMP check, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 06/20] monitor: Use traditional command interface for HMP device_add, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 05/20] monitor: Use traditional command interface for HMP drive_del, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 14/20] monitor: Rename handle_user_command() to handle_hmp_command(), Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 07/20] monitor: Use trad. command interface for HMP pcie_aer_inject_error, Markus Armbruster, 2015/05/22