From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33003) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zzs7D-0002Hq-3s for qemu-devel@nongnu.org; Fri, 20 Nov 2015 15:17:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zzs78-00038L-46 for qemu-devel@nongnu.org; Fri, 20 Nov 2015 15:17:03 -0500 Received: from mail-wm0-x22c.google.com ([2a00:1450:400c:c09::22c]:35414) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zzs77-00038D-Tr for qemu-devel@nongnu.org; Fri, 20 Nov 2015 15:16:58 -0500 Received: by wmdw130 with SMTP id w130so33054166wmd.0 for ; Fri, 20 Nov 2015 12:16:57 -0800 (PST) Sender: Paolo Bonzini References: <1448010269-21694-1-git-send-email-pbonzini@redhat.com> <1448010269-21694-2-git-send-email-pbonzini@redhat.com> <564F651A.6090107@redhat.com> From: Paolo Bonzini Message-ID: <564F7FB4.8000301@redhat.com> Date: Fri, 20 Nov 2015 21:16:53 +0100 MIME-Version: 1.0 In-Reply-To: <564F651A.6090107@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/4] qjson: replace QString in JSONLexer with GString List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: armbru@redhat.com On 20/11/2015 19:23, Eric Blake wrote: > On 11/20/2015 02:04 AM, Paolo Bonzini wrote: >> JSONLexer only needs a simple resizable buffer. json-streamer.c >> can allocate memory for each token instead of relying on reference >> counting of QStrings. >> >> Signed-off-by: Paolo Bonzini >> --- >> include/qapi/qmp/json-lexer.h | 7 +++---- >> include/qapi/qmp/json-streamer.h | 1 + >> qobject/json-lexer.c | 22 ++++++++-------------- >> qobject/json-streamer.c | 10 +++++----- >> 4 files changed, 17 insertions(+), 23 deletions(-) >> > >> --- a/include/qapi/qmp/json-streamer.h >> +++ b/include/qapi/qmp/json-streamer.h >> @@ -14,6 +14,7 @@ >> #ifndef QEMU_JSON_STREAMER_H >> #define QEMU_JSON_STREAMER_H >> >> +#include >> #include "qapi/qmp/qlist.h" >> #include "qapi/qmp/json-lexer.h" >> > > Spurious hunk? If not, should mention in the commit message why this is > needed. Does "because otherwise it doesn't compile" count? :) It was relying on json-lexer.h indirectly including stdint.h. I don't think it ought to be mentioned in the commit message. Paolo >> +++ b/qobject/json-streamer.c >> @@ -12,6 +12,7 @@ >> */ >> >> #include "qapi/qmp/qlist.h" >> +#include "qapi/qmp/qstring.h" >> #include "qapi/qmp/qint.h" >> #include "qapi/qmp/qdict.h" >> #include "qemu-common.h" >> @@ -21,13 +22,13 @@ >> #define MAX_TOKEN_SIZE (64ULL << 20) >> #define MAX_NESTING (1ULL << 10) >> >> -static void json_message_process_token(JSONLexer *lexer, QString *token, JSONTokenType type, int x, int y) >> +static void json_message_process_token(JSONLexer *lexer, GString *input, JSONTokenType type, int x, int y) > > Worth wrapping the long line while touching it? > > Otherwise looks like a straightforward conversion. If the commit > message is fixed, you can add > > Reviewed-by: Eric Blake >