[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 05/15] qjson: add "opaque" field to JSONMessagePar
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [RFC 05/15] qjson: add "opaque" field to JSONMessageParser |
Date: |
Tue, 19 Sep 2017 15:55:41 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 09/14/2017 02:50 AM, Peter Xu wrote:
> It'll be passed to emit() as well when it happens.
>
> Signed-off-by: Peter Xu <address@hidden>
> ---
> include/qapi/qmp/json-streamer.h | 8 ++++++--
> monitor.c | 7 ++++---
> qga/main.c | 5 +++--
> qobject/json-streamer.c | 7 +++++--
> qobject/qjson.c | 5 +++--
> tests/libqtest.c | 5 +++--
> 6 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/include/qapi/qmp/json-streamer.h
> b/include/qapi/qmp/json-streamer.h
> index 00d8a23..b83270c 100644
> --- a/include/qapi/qmp/json-streamer.h
> +++ b/include/qapi/qmp/json-streamer.h
> @@ -25,16 +25,20 @@ typedef struct JSONToken {
>
> typedef struct JSONMessageParser
> {
> - void (*emit)(struct JSONMessageParser *parser, GQueue *tokens);
> + void (*emit)(struct JSONMessageParser *parser, GQueue *tokens, void
> *opaque);
> JSONLexer lexer;
> int brace_count;
> int bracket_count;
> GQueue *tokens;
> uint64_t token_size;
> + /* To be passed in when emit(). */
Reads awkwardly, better might be: /* Passed to emit() */
I might group void *opaque right next to emit, rather than separated by
the rest of the struct, to make it obvious they are related, at which
point the comment isn't necessary.
> + void *opaque;
> } JSONMessageParser;
>
> void json_message_parser_init(JSONMessageParser *parser,
> - void (*func)(JSONMessageParser *, GQueue *));
> + void (*func)(JSONMessageParser *, GQueue *,
> + void *opaque),
Inconsistent to name only one of the three parameters in the inner
function pointer type for the outer parameter 'func'. I wonder if a
typedef would make things more legible.
> + void *opaque);
>
> +++ b/qobject/json-streamer.c
> @@ -102,18 +102,21 @@ out_emit:
> */
> tokens = parser->tokens;
> parser->tokens = g_queue_new();
> - parser->emit(parser, tokens);
> + parser->emit(parser, tokens, parser->opaque);
> parser->token_size = 0;
> }
>
> void json_message_parser_init(JSONMessageParser *parser,
> - void (*func)(JSONMessageParser *, GQueue *))
> + void (*func)(JSONMessageParser *,
> + GQueue *, void *opaque),
Again, inconsistent that you named only 1 of the three inner parameters.
Overall, the idea looks reasonable.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [RFC 00/15] QMP: out-of-band (OOB) execution support, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 01/15] char-io: fix possible race on IOWatchPoll, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 02/15] qobject: allow NULL for qstring_get_str(), Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 03/15] qobject: introduce qobject_to_str(), Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 04/15] monitor: move skip_flush into monitor_data_init, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 05/15] qjson: add "opaque" field to JSONMessageParser, Peter Xu, 2017/09/14
- Re: [Qemu-devel] [RFC 05/15] qjson: add "opaque" field to JSONMessageParser,
Eric Blake <=
- [Qemu-devel] [RFC 06/15] monitor: move the cur_mon hack deeper for QMP, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 07/15] monitor: unify global init, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 08/15] monitor: create IO thread, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 09/15] monitor: allow to use IO thread for parsing, Peter Xu, 2017/09/14