From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50908) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtdCt-0005NJ-JK for qemu-devel@nongnu.org; Tue, 03 Nov 2015 10:09:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtdCo-0000wO-FX for qemu-devel@nongnu.org; Tue, 03 Nov 2015 10:09:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33112) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtdCo-0000vu-8k for qemu-devel@nongnu.org; Tue, 03 Nov 2015 10:09:02 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id B536B42E5A9 for ; Tue, 3 Nov 2015 15:09:01 +0000 (UTC) From: Markus Armbruster References: <5633C8EC.8030309@redhat.com> <874mh44wvs.fsf@blackfin.pond.sub.org> <56378572.5020203@redhat.com> <87egg8nro5.fsf@blackfin.pond.sub.org> <5637C2B3.6090609@redhat.com> <87egg7lffd.fsf@blackfin.pond.sub.org> <20151103081917.1d348ad1@redhat.com> <5638B8DE.6020709@redhat.com> <20151103084643.26d71554@redhat.com> <5638BC77.5010102@redhat.com> <20151103092659.126ab97c@redhat.com> Date: Tue, 03 Nov 2015 16:08:58 +0100 In-Reply-To: <20151103092659.126ab97c@redhat.com> (Luiz Capitulino's message of "Tue, 3 Nov 2015 09:26:59 -0500") Message-ID: <87d1vr86id.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] RFC: libyajl for JSON List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Paolo Bonzini , "Dr. David Alan Gilbert" , "qemu-devel@nongnu.org" Luiz Capitulino writes: > On Tue, 3 Nov 2015 14:53:59 +0100 > Paolo Bonzini wrote: > >> On 03/11/2015 14:46, Luiz Capitulino wrote: >> >> > Can you explain why that would make sense? :) (Especially since there >> >> > is another extension---JSON5---that does exactly what we're doing, so it >> >> > probably wasn't that stupid an idea). >> > Let's be pragmatic. *If* this is the only issue stopping us from >> > dropping our own parser in favor of something more widely used and >> > *if* libvirt doesn't make use of the feature, it's something we >> > should strongly consider. >> >> I'm not sure what's so bad about our parser that makes it worthwhile to: > > It's not that it's bad. It's about the advantages of dropping hundreds of > lines of NIH code and switching to something more widely used. I wish we would've / could've avoided NIH back then, but I'm not sure getting rid of this piece of NIH now is worthwhile. json-{lexer,parser,streamer}.[ch] together have 949 SLOC. I'm not counting tests, because we'd most likely keep them anyway. This is less than 0.1% of the QEMU source code :) Maintenance hasn't been costing us a fortune exactly: 40 commits in six years. I'm annoyed by its relative shoddiness, but the prospect of having to fix it up isn't something that keeps me up at night. > Also, > any maintenance time we spend on libyajl will also be automatically > enjoyed by libvirt which is excellent. Excellent indeed *if* upstream is responsive. > On the other hand, I don't want to push too hard for it because I do > recognize that switching has a cost and I won't be able to help with > that myself. > >> 1) uglify all tests and make them inconsistent with the QAPI schemas, >> which also uses single-quoted strings > > This doesn't seem hard to fix, we could pre-process the test files, > say in Python, to add the needed escaping. The test files are actually C code... sure you want to mangle C code in Python? >> 2) waste time finding a replacement for % interpolation (the best >> replacement here would be to rewrite the tests in Python IMHO, but >> that's not a small ask) > > Is this only used by tests? No. > Can you give an example of this feature? Yes: static QDict *build_qmp_error_dict(Error *err) { QObject *obj; obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %s } }", ErrorClass_lookup[error_get_class(err)], error_get_pretty(err)); return qobject_to_qdict(obj); } Builds a QDict with a single key "error". Its value is a QDict with key "class", value ErrorClass_lookup[error_get_class(err)], and key "desc", value error_get_pretty(err), where "value" really means the C string quoted for JSON and wrapped in a QString. As I wrote elsewhere in the thread, we could build this by hand. Much less readable, but that might be tolerable, as the feature isn't widely used. I'm not thrilled about it, though. It's too easy to forget the quoting. To find more examples, try "git-grep _json[fv]". >> Just let's remove the weird (to not say worse) usage of QDict/QList to >> store tokens... Any replacement effort will have to compete on merits with fixing up what we have.