Re: [PATCH V4 09/24] libxl_json: introduce parser functions for builtin types

[Wei Liu <wei.liu2@citrix.com>]

On Tue, May 06, 2014 at 03:45:47PM +0100, Wei Liu wrote:
> On Tue, May 06, 2014 at 03:35:27PM +0100, Anthony PERARD wrote:
> [...]
> > > +/* Macro to generate:
> > > + *  libxl__uint8_parse_json
> > > + *  libxl__uint16_parse_json
> > > + *  libxl__uint32_parse_json
> > > + *  libxl__uint64_parse_json
> > > + */
> > > +#define PARSE_UINT(width)                                               \
> > > +    int libxl__uint ## width ## _parse_json(libxl__gc *gc,              \
> > > +                                            const libxl__json_object *o,\
> > > +                                            void *p)                    \
> > > +    {                                                                   \
> > > +        long long i;                                                    \
> > > +                                                                        \
> > > +        if (!libxl__json_object_is_integer(o))                          \
> > > +            return ERROR_FAIL;                                          \
> > > +                                                                        \
> > > +        i = libxl__json_object_get_integer(o);                          \
> > > +                                                                        \
> > > +        if (i > UINT ## width ## _MAX)                                  \
> > > +            return ERROR_FAIL;                                          \
> > 
> > My guest is this will always be false for uint64 and maybe for uint32.
> > The value return by get_interger can only be <= LLONG_MAX which I
> > suppose is still < UINT64_MAX.
> > 
> 
> I was just being lazy about it.
> 
> > Also, 'i' might be < 0.
> > 
> 
> Phew, I knew this but somehow I thought it was OK. :-(
> 
> So the two things combined, the check should be
> 
>    (i > 0 && i < UINT ## width ## _MAX)
> 
> What do you think?
> 
> > If json contain a value > LLONG_MAX, the value will be store as a string
> > with the type number.
> > 
> 
> This is also true, but I coded like this on purpose. You won't be able
> to convert that string anyway because that's why it's stored as string
> in the first place. We should just return ERROR_FAIL in this case, which
> we do already with the check libxl__json_object_is_integer.
> 

Of course with the sole exception of uint64_t parser. This is a special
case that needs to be handled.

Wei.

> Wei.