[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 01/14] Introduce a simple API for handling JSON data



On Thu, Dec 03, 2009 at 12:41:24PM +0000, Daniel P. Berrange wrote:
> On Thu, Dec 03, 2009 at 01:11:19PM +0100, Daniel Veillard wrote:
> > On Thu, Nov 26, 2009 at 06:27:19PM +0000, Daniel P. Berrange wrote:
> > > 
> > > This uses the YAJL library for parsing/formatting from
> > > 
> > >  http://lloyd.github.com/yajl/
> > 
> > > * src/util/json.h, src/util/json.c: Data structures and APIs
> > >   for representing JSON data, and parsing/formatting it
> > > * configure.in: Add check for yajl library
> > > * libvirt.spec.in: Add build requires for yajl
> > > * src/Makefile.am: Add json.c/h
> > > * src/libvirt_private.syms: Export JSON symbols to drivers
> > 
> >   hum ... that becomes an external library, one more dependancy
> > on the other hand we don't have to track bug fixes.... how widely used
> > is yajl ?
> 
> It is very unclear - JSON doesn't really appear to be used much
> from C at all - it is mostly something that python/perl/javascript
> people use. There are six parsers listed on http://json.org/ and
> I ended up picking yajl because it was a very nicely designed API
> that integrated well with what I wanted.

  http://lloyd.github.com/yajl/

The licence is BSD ... but with what looks like an advertizing clause
see point 2. below:

------------------------------------------------------------------------
 Copyright 2007-2009, Lloyd Hilaiel.
 
 Redistribution and use in source and binary forms, with or without
 modification, are permitted provided that the following conditions are
 met:
 
  1. Redistributions of source code must retain the above copyright
     notice, this list of conditions and the following disclaimer.
 
  2. Redistributions in binary form must reproduce the above copyright
     notice, this list of conditions and the following disclaimer in
     the documentation and/or other materials provided with the
     distribution.
 
  3. Neither the name of Lloyd Hilaiel nor the names of its
     contributors may be used to endorse or promote products derived
     from this software without specific prior written permission.
 
 THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
 IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
 WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT,
 INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
 (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
 SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
 STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
 IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 POSSIBILITY OF SUCH DAMAGE.
------------------------------------------------------------------------


> >   yajl is not in F12, did someone add it to rawhide already ?
> 
> No, its not in Fedora at all. If we go for this patch, then I will
> have to prepare an yajl RPM for Fedora.

  humpf ...

> > > +#define ReportError(conn, code, fmt...)                                 \
> > > +    virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__,           \
> > > +                         __FUNCTION__, __LINE__, fmt)
> > > +
> > > +
> > [...]
> > > +static virJSONValuePtr virJSONValueNewNumber(const char *data)
> > > +{
> > > +    virJSONValuePtr val;
> > > +
> > > +    if (VIR_ALLOC(val) < 0)
> > > +        return NULL;
> > > +
> > > +    val->type = VIR_JSON_TYPE_NUMBER;
> > > +    if (!(val->data.number = strdup(data))) {
> > > +        VIR_FREE(val);
> > > +        return NULL;
> > > +    }
> > 
> >   so we keep numbers as their string value ... surprizing but
> > maybe that's the most flexible for our use.
> 
> The reason for this is that at a low level JSON does not define 
> whether a number is an int, floating point, etc, etc. Thus when
> we parse numbers we just get a string.

  okay

> Only the application (in this case libvirt's QEMU driver) knows
> what number format is applicable for a particular context. So
> when the libvirt QEMU monitor code wants to set/get a number,
> it will use one of the wrapper methods like
> 
>   virJSONValueNewNumberInt(int value)
>   virJSONValueNewNumberLong(long long value)
>   virJSONValueNewNumberDouble(double value)

  okay ... I start wondering if I should not write yet another parser,
after all it's trivial compared to XML or HTML from libxml2 ...

> > > +
> > > +/* XXX add an incremental streaming parser - yajl trivially supports it */
> > 
> >   what would be the point ? I doubt there is a memory problem, it's more
> > the incremental aspect I suppose.
> 
> It is all about how we decent the message boundaries. The current
> QEMU code will always terminate a JSON message with a '\r\n', so
> we can easily detect boundaries without needing to parse the data.
> 
> If QEMU developers suddenly change their mind and remove the '\r\n',
> we would need an incremental streaming parser, so we can detect 
> where the message boundaries are.

  Ah, I see, this is tricking so many people using XML, as the
parser can't tell when the document is finished (it accepts unlimited
extra blanks/comment/PI after the main emelemt)

> Hopefully they won't change their minds :-)

  :-)

> >   ACK, it' weird there ain't any JSON C API yet in F12, there is one for
> > nearly all high level language but noone seems to have imposed a good C
> > one. so now we are betting on yajl, maybe another one will prevail but
> > since we have a good decoupling, fine by me,
> 
> The only other option I see is to actually include the YAJL parsing/formatting
> code in the libvirt source tree, but that is rather evil in itself :-)

  yes if it ships as a standalone library then we should ot do this.
The alternative would be to write another one, use it only internally
and possibly separate it later or stuff it in libxml2 or ...

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]