[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 12/24] qobject: Propagate parse errors through q
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 12/24] qobject: Propagate parse errors through qobject_from_json() |
Date: |
Tue, 28 Feb 2017 13:19:38 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/27/2017 05:20 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> block.c | 2 +-
> include/qapi/qmp/qjson.h | 5 +--
> monitor.c | 2 +-
> qobject/qjson.c | 4 +--
> tests/check-qjson.c | 62
> +++++++++++++++++++-------------------
> tests/test-visitor-serialization.c | 2 +-
> 6 files changed, 39 insertions(+), 38 deletions(-)
>
As with 8/24, this would be a good place for the commit message to
mention that this patch adds the parameter and mechanically adjusts
callers with minimal semantic changes, but that later patches will take
advantage of the parameter.
> +++ b/include/qapi/qmp/qjson.h
> @@ -17,8 +17,9 @@
> #include "qapi/qmp/qobject.h"
> #include "qapi/qmp/qstring.h"
>
> -QObject *qobject_from_json(const char *string);
> -QObject *qobject_from_jsonf(const char *string, ...) GCC_FMT_ATTR(1, 2);
> +QObject *qobject_from_json(const char *string, Error **errp);
The real change here, vs.
> +QObject *qobject_from_jsonf(const char *string, ...)
> + GCC_FMT_ATTR(1, 2);
formatting. Should the formatting be hoisted earlier in the series,
when you first touch qobject_from_jsonv?
> QObject *qobject_from_jsonv(const char *string, va_list *ap, Error **errp)
> GCC_FMT_ATTR(1, 0);
This makes qobject_from_jsonf() and qobject_from_jsonv() asymmetric
(only one of the two can report errors); and looks a bit weird to have a
va_list not as the last argument (as it is, a 'va_list *' argument is
already weird). If symmetry is important, we can put errp prior to the
.../ap argument (then both forms have an errp pointer). But I don't
think it matters in the context of this series. If you DO change it,
though, then 8/24 would be the place to tweak it.
At one point, I posted a series that removed all uses of
qobject_from_json[fv] (leaving just qobject_from_json); at the time,
there was not a strong opinion on whether the series was worthwhile, but
if we want it, I'm fine rebasing on top of this. (One argument in favor
of my series would be getting rid of the weird 'va_list *' argument).
> +++ b/qobject/qjson.c
> @@ -50,9 +50,9 @@ QObject *qobject_from_jsonv(const char *string, va_list
> *ap, Error **errp)
> return state.result;
> }
>
> -QObject *qobject_from_json(const char *string)
> +QObject *qobject_from_json(const char *string, Error **errp)
> {
> - return qobject_from_jsonv(string, NULL, NULL);
> + return qobject_from_jsonv(string, NULL, errp);
Of course, if you rebase the series to rearrange where errp appears in
relation to va_list, then be sure this changes to match (the compiler
may or may not flag it, if va_list looks too much like void*).
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
- [Qemu-devel] [PATCH 11/24] test-qobject-input-visitor: Abort earlier on bad test input, (continued)
- [Qemu-devel] [PATCH 11/24] test-qobject-input-visitor: Abort earlier on bad test input, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 01/24] test-qemu-opts: Cover qemu_opts_parse() of "no", Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 16/24] monitor: Assert qmp_schema_json[] is sane, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 15/24] test-visitor-serialization: Pass &error_abort to qobject_from_json(), Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 12/24] qobject: Propagate parse errors through qobject_from_json(), Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 20/24] docs/qapi-code-gen.txt: Clarify naming rules, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 13/24] block: More detailed syntax error reporting for JSON filenames, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 05/24] test-keyval: Cover use with qobject input visitor, Markus Armbruster, 2017/02/27
- [Qemu-devel] [PATCH 14/24] check-qjson: Test errors from qobject_from_json(), Markus Armbruster, 2017/02/27