Re: [Xen-devel] Re: [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Jul 1, 2011 at 09:14, Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx> wrote:
>> +/* from libxl_json */
>> +typedef enum {
>> + Â ÂJSON_ERROR,
>> + Â ÂJSON_NULL,
>> + Â ÂJSON_TRUE,
>> + Â ÂJSON_FALSE,
>> + Â ÂJSON_INTEGER,
>> + Â ÂJSON_DOUBLE,
>> + Â ÂJSON_STRING,
>> + Â ÂJSON_MAP,
>> + Â ÂJSON_ARRAY,
>> + Â ÂJSON_ANY
>> +} libxl__json_node_type;
>
> Could be internal IDL? Maybe no much point if you aren't using the
> to_string functions etc?

The to_string function could be use for debug messages. But that all.

>> +
>> +typedef struct libxl__json_object {
>> + Â Âlibxl__json_node_type type;
>> + Â Âunion {
>> + Â Â Â Âlong i;
>> + Â Â Â Âdouble d;
>> + Â Â Â Âconst char *string;
>
> Is it really const? Seems to be malloc/freed?

Yes, the string belong only to this structure. In that case, there is no const ?

>> [...]+#ifdef DEBUG_ANSWER
>> +# Âdefine DEBUG_GEN_ALLOC(h) \
>> + Â Âif (h == NULL) { \
>> + Â Â Â Âyajl_gen_config conf = { 1, " Â" }; \
>> + Â Â Â Âh = yajl_gen_alloc(&conf, NULL); \
>> + Â Â}
>
> All the callers of these macros use ctx->g. I think you could push the
> ->g down into the macros.

Yes, maybe better, specially for the case of the _alloc where the
parameter is modified.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel