qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Graf <agraf@suse.de>
To: Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org
Cc: amit.shah@redhat.com, pbonzini@redhat.com, afaerber@suse.de,
	quintela@redhat.com
Subject: Re: [Qemu-devel] [PATCH v3 2/5] QJSON: Add JSON writer
Date: Tue, 06 Jan 2015 22:16:39 +0100	[thread overview]
Message-ID: <54AC50B7.6060108@suse.de> (raw)
In-Reply-To: <54AC02F4.40403@redhat.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 06.01.15 16:44, Eric Blake wrote:
> On 12/26/2014 07:42 AM, Alexander Graf wrote:
>> To support programmatic JSON assembly while keeping the code that
>> generates it readable, this patch introduces a simple JSON
>> writer. It emits JSON serially into a buffer in memory.
>> 
>> The nice thing about this writer is its simplicity and low memory
>> overhead. Unlike the QMP JSON writer, this one does not need to
>> spawn QObjects for every element it wants to represent.
>> 
>> This is a prerequisite for the migration stream format
>> description generator.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> 
>> ---
>> 
>> v2 -> v3:
>> 
>> - QOMify the QJSON object, makes for easier destruction --- 
>> include/qjson.h |  3 ++- qjson.c         | 39
>> ++++++++++++++++++++++++++++++++++++--- 2 files changed, 38
>> insertions(+), 4 deletions(-)
>> 
>> diff --git a/include/qjson.h b/include/qjson.h index
>> 8f8c145..7c54fdf 100644 --- a/include/qjson.h +++
>> b/include/qjson.h @@ -4,7 +4,7 @@ * Copyright Alexander Graf * *
>> Authors: - *  Alexander Graf <agraf@suse.de + *  Alexander Graf
>> <agraf@suse.de>
> 
> Umm, this should be squashed into 1/5.

Honestly, I think the whole patch should've gotten squashed into 1/5.
I'm not quite sure what went wrong here :).

> 
>> @@ -85,9 +88,7 @@ const char *qjson_get_str(QJSON *json)
>> 
>> QJSON *qjson_new(void) { -    QJSON *json = g_new(QJSON, 1); -
>> json->str = qstring_from_str("{ "); -    json->omit_comma =
>> true; +    QJSON *json = (QJSON *)object_new(TYPE_QJSON); return
>> json;
> 
> This undoes the dangling object that I complained about on patch
> 1; maybe there's some more squashing to do?...
> 
> 
>> +static void qjson_initfn(Object *obj) +{ +    QJSON *json =
>> (QJSON *)object_dynamic_cast(obj, TYPE_QJSON); +
>> assert(json); + +    json->str = qstring_from_str("{ "); +
>> json->omit_comma = true;
> 
> ...or is it still an incomplete object, just now in a different
> location?

The idea is that you call finish and only then receive the string. I
don't think doing that from get_str would be too much of an
improvement - it'd still be wrong if we're in the middle of assembling
an object.


Alex
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJUrFC2AAoJECszeR4D/txgFUoP/A79amlLyEBoD7/G1jHttwS2
sYKst8ZM+v//dB5Xe+DzU53XgflAkpUoz27zSveluD1A3YQ3zpFYraujaewSn++m
sJZS/7bS9WND0vhKdGj41iJXdjxSlHqcdrsKRqYR4MaaTjH8eVoabECS+CcdK0xc
R+x08gSUI22+tCKPnudRMm3mlVqJ5O+1KbTYShQQlP5go1p6UN32LONgZ4u9oJPL
uQ1vkkrHleHa+RoW4HzRCSO+LQuY/y8CMV1ufS50pILSoX5C/jDdOzqvpc3fR9Ba
X1dWVATsF8gk+Hu4sNtWAqiPSsSGg/V0CnBASRYwt/skoNtXoZ++/UWr8B+F/IKc
h2UCdO64TlbbDtGYGn/4ZQuUIsGLPvm0yN8J1LshVt85wWJaNe2dw8hirxjzCFoB
A6YnueXbaxkHlQkVXL0u2E/EUnmYlRl8JauFBxVDuw6MQecQtc7MIFS0zr8XyKDt
IxspJpOKGXAuU7oK+1kmy2aOlMJCVwgdthCv9qDaQl39pTpyF38Jf8kOAFWSkz+C
bQqq4EZrI4vMlHcwY4a06YnFH7Ek8Q7YZdFj/W2CIJKsIMh0k79mFjm4r54JrIUF
UTXoR/DuZd/zRGLBnj8PoBsfiXk3ZGJCtAyrlns2T32sEbiJmL3lqggiyKzh3+Dd
S9NeyhQvJ/xvYPQ89N7p
=Zod1
-----END PGP SIGNATURE-----

  reply	other threads:[~2015-01-06 21:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-26 14:42 [Qemu-devel] [PATCH v3 0/5] Migration Deciphering aid Alexander Graf
2014-12-26 14:42 ` [Qemu-devel] [PATCH v3 1/5] QJSON: Add JSON writer Alexander Graf
2015-01-06 15:41   ` Eric Blake
2015-01-06 21:39     ` Alexander Graf
2014-12-26 14:42 ` [Qemu-devel] [PATCH v3 2/5] " Alexander Graf
2015-01-06 15:44   ` Eric Blake
2015-01-06 21:16     ` Alexander Graf [this message]
2014-12-26 14:42 ` [Qemu-devel] [PATCH v3 3/5] qemu-file: Add fast ftell code path Alexander Graf
2015-01-06 15:46   ` Eric Blake
2014-12-26 14:42 ` [Qemu-devel] [PATCH v3 4/5] migration: Append JSON description of migration stream Alexander Graf
2015-01-06 15:56   ` Eric Blake
2015-01-06 21:25     ` Alexander Graf
2015-01-20 10:30   ` Amit Shah
2014-12-26 14:42 ` [Qemu-devel] [PATCH v3 5/5] Add migration stream analyzation script Alexander Graf
2015-01-06 16:05   ` Eric Blake
2015-01-06 21:29     ` Alexander Graf
2015-01-20 10:31 ` [Qemu-devel] [PATCH v3 0/5] Migration Deciphering aid Amit Shah
2015-01-20 10:54   ` Alexander Graf
2015-01-21  6:05     ` Amit Shah

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54AC50B7.6060108@suse.de \
    --to=agraf@suse.de \
    --cc=afaerber@suse.de \
    --cc=amit.shah@redhat.com \
    --cc=eblake@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).