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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, May 06, 2014 at 04:14:53PM +0100, Anthony PERARD wrote:
> On Tue, May 06, 2014 at 03:49:06PM +0100, Wei Liu wrote:
> > 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?
>
> 0 is also valid ;), and I think UINTX_MAX is valid too,
> so (i >= 0 && i <= UINT ## width ## _MAX) should do.
> Or:
> if (i < 0 || i > UINT ## width ## _MAX)
>   return ERROR_FAIL;
>
>
> > > > 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.
>
> Right.
>

For your convenient I paste the relevant code snippet here:

+/* Macro to generate:
+ *  libxl__uint8_parse_json
+ *  libxl__uint16_parse_json
+ *  libxl__uint32_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 < 0 || i > UINT ## width ## _MAX)                         \
+            return ERROR_FAIL;                                          \
+                                                                        \
+        *((uint ## width ## _t *)p) = i;                                \
+                                                                        \
+        return 0;                                                       \
+    }
+
+PARSE_UINT(8);
+PARSE_UINT(16);
+PARSE_UINT(32);
+
+int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
+                             void *p)
+{
+    if (!libxl__json_object_is_integer(o) &&
+        !libxl__json_object_is_number(o))
+        return ERROR_FAIL;
+
+    if (libxl__json_object_is_integer(o)) {
+        long long i = libxl__json_object_get_integer(o);
+
+        if (i < 0)
+            return ERROR_FAIL;
+
+        *((uint64_t *)p) = i;
+    } else {
+        const char *s;
+        unsigned long long i;
+        int saved_errno = errno;
+
+        s = libxl__json_object_get_number(o);
+
+        errno = 0;
+        i = strtoull(s, NULL, 10);
+
+        if (i == ULLONG_MAX && errno == ERANGE)
+            return ERROR_FAIL;
+
+        errno = saved_errno;
+        *((uint64_t *)p) = i;
+    }
+
+    return 0;
+}

> --
> Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel