[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC V2 06/10] libxl_json: allow basic JSON type objects generation
On Wed, Apr 23, 2014 at 11:30:03AM +0100, Ian Campbell wrote: > On Wed, 2014-04-23 at 11:15 +0100, Wei Liu wrote: > > On Tue, Apr 22, 2014 at 04:22:20PM +0100, Ian Campbell wrote: > > > On Thu, 2014-04-17 at 12:13 +0100, Wei Liu wrote: > > > > The original logic is that basic JSON types (number, string and null) > > > > must be an element of JSON map or array. This assumption doesn't hold > > > > true anymore when we need to return basic JSON types. > > > > > > > > Returning basic JSON types is required for parsing number, string and > > > > null objects back into libxl__json_object. > > > > > > > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx> > > > > --- > > > > tools/libxl/libxl_json.c | 40 > > > > ++++++++++++++++++++++++++++++++-------- > > > > 1 file changed, 32 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c > > > > index e2d5dbe..6b2946d 100644 > > > > --- a/tools/libxl/libxl_json.c > > > > +++ b/tools/libxl/libxl_json.c > > > > @@ -629,8 +629,14 @@ static int json_callback_null(void *opaque) > > > > > > > > obj = libxl__json_object_alloc(ctx->gc, JSON_NULL); > > > > > > > > - if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == > > > > -1) { > > > > - return 0; > > > > + if (ctx->current) { > > > > + if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) > > > > == -1) { > > > > + return 0; > > > > + } > > > > + } > > > > + > > > > + if (ctx->head == NULL) { > > > > + ctx->head = obj; > > > > > > It seems that this pattern is now pretty much universal on all callers > > > of libxl__json_object_append_to. Perhaps much of this functionality > > > should be pushed down into the helper (which might now need to take ctx > > > and not ctx->current). > > > > > > > But not every new allocated object needs to be appended to some other > > object, so pushing that functionality to libxl__json_object_append_to is > > not correct. > > ??? > > AFAICT every call of libxl__json_object_append_to after this patch now > has exactly the same boiler plate around it. The need append or not is a > property of the current context (in a map) not of the current object > being handled. > But setting ctx->head doesn't really belong to append_to does it? The semantic looks wrong to me. Are you suggesting me change the semantic of libxl__json_object_append_to to "append this object to current context" instead of "append this object to the map or array pointed to by 'current'"? Wei. > > > The existing callers seem to all set ctx->current too, just before the > > > ctx->head == NULL check. Why don't these versions need that too? > > > > > > > Because "current" is used to reference to "the array or map that we're > > currently in", so that we can call > > libxl__json_object_append_to(current). > > > > Not very useful to set "current" for basic types because no other > > objects can be appended to them. > > OK, I think this bit could be left in the relevant callers rather than > being made generic. Or it could be a parameter to object_append_to (e.g. > bool_t is_a_container, or pass the JSON_*). > > Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |