[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/20] monitor: Limit QError use to command hand
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 13/20] monitor: Limit QError use to command handlers |
Date: |
Fri, 22 May 2015 16:38: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:
> The previous commits narrowed use of QError to handle_qmp_command()
> and its helpers monitor_protocol_emitter(), build_qmp_error_dict().
> Narrow it further to just the command handler call: instead of
> converting Error to QError throughout handle_qmp_command(), convert
> the QError gotten from the command handler to Error, and switch the
> helpers from QError to Error.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> monitor.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 71ca03f..65ef400 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -391,19 +391,19 @@ static void monitor_json_emitter(Monitor *mon, const
> QObject *data)
> QDECREF(json);
> }
>
> -static QDict *build_qmp_error_dict(const QError *err)
> +static QDict *build_qmp_error_dict(Error *err)
> {
> QObject *obj;
>
> - obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %p } }",
> - ErrorClass_lookup[err->err_class],
> - qerror_human(err));
> + obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %s } }",
> + ErrorClass_lookup[error_get_class(err)],
> + error_get_pretty(err));
You're getting rid of one of three uses of '_jsonf.*%p' in the tree (two
in monitor.c, one in tests/check-qjson.c) [I didn't check for multi-line
instances where the format string was on a different line than the
function call]. Is it worth completely getting rid of the %p formatter,
and simplifying qobject/json-*.c accordingly, in a later patch?
[oh, and I hate that I could not find documentation on the format
strings accepted by qobject_from_jsonf; it took me forever to track down
that %i correlated to bool, when I was doing my series for "qobject" Use
'bool' for qbool]
> @@ -4984,13 +4984,12 @@ static void handle_qmp_command(JSONMessageParser
> *parser, QList *tokens)
> obj = json_parser_parse(tokens, NULL);
> if (!obj) {
> // FIXME: should be triggered in json_parser_parse()
> - qerror_report(QERR_JSON_PARSING);
> + error_set(&local_err, QERR_JSON_PARSING);
> goto err_out;
Is this FIXME still valid, or are we reaching the point where it could
be replaced by an assert(obj) in a later patch?
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 01/20] monitor: Drop broken, unused asynchronous command interface, (continued)
- [Qemu-devel] [PATCH 09/20] monitor: Propagate errors through qmp_check_client_args(), Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 18/20] monitor: Turn int command_mode into bool in_command_mode, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 19/20] monitor: Rename monitor_ctrl_mode() to monitor_is_qmp(), Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 13/20] monitor: Limit QError use to command handlers, Markus Armbruster, 2015/05/22
- Re: [Qemu-devel] [PATCH 13/20] monitor: Limit QError use to command handlers,
Eric Blake <=
- [Qemu-devel] [PATCH 16/20] monitor: Unbox Monitor member mc and rename to qmp, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 11/20] monitor: Wean monitor_protocol_emitter() off mon->error, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 15/20] monitor: Rename monitor_control_read(), monitor_control_event(), Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 20/20] monitor: Change return type of monitor_cur_is_qmp() to bool, Markus Armbruster, 2015/05/22