From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony PERARD Subject: Re: [PATCH V2 4/9] libxl_qmp: Introduces helpers to create an argument list. Date: Tue, 25 Sep 2012 15:45:50 +0100 Message-ID: <5061C39E.3070904@citrix.com> References: <1347906148-9606-1-git-send-email-anthony.perard@citrix.com> <1347906148-9606-5-git-send-email-anthony.perard@citrix.com> <1348563254.3452.120.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1348563254.3452.120.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Ian Jackson , Xen Devel List-Id: xen-devel@lists.xenproject.org On 09/25/2012 09:54 AM, Ian Campbell wrote: > On Mon, 2012-09-17 at 19:22 +0100, Anthony PERARD wrote: >> Those functions will be used to create a "list" of parameters that contain more >> than just strings. This list is converted by qmp_send to a string to be sent to >> QEMU. >> >> Those functions will be used in the next two patches, so right now there are >> not compiled. >> >> Signed-off-by: Anthony PERARD >> --- >> tools/libxl/libxl_qmp.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 53 insertions(+) >> >> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c >> index 07a8bd5..1dd5c6c 100644 >> --- a/tools/libxl/libxl_qmp.c >> +++ b/tools/libxl/libxl_qmp.c >> @@ -623,6 +623,59 @@ static void qmp_free_handler(libxl__qmp_handler *qmp) >> free(qmp); >> } >> >> +#if 0 >> +/* >> + * QMP Parameters Helpers >> + * Those functions does not use the gc, because of the internal usage of >> + * flexarray that does not support it. >> + * The allocated *param need to be free with libxl__json_object_free(gc, param) >> + */ >> +static void qmp_parameters_common_add(libxl__gc *gc, >> + libxl__json_object **param, >> + const char *name, >> + libxl__json_object *obj) >> +{ >> + libxl__json_map_node *arg = NULL; >> + >> + if (!*param) { >> + *param = libxl__json_object_alloc(NOGC, JSON_MAP); >> + } >> + >> + arg = libxl__zalloc(NOGC, sizeof(*arg)); >> + >> + arg->map_key = libxl__strdup(NOGC, name); >> + arg->obj = obj; >> + >> + flexarray_append((*param)->u.map, arg); >> +} >> + >> +static void qmp_parameters_add_string(libxl__gc *gc, >> + libxl__json_object **param, >> + const char *name, const char *argument) >> +{ >> + libxl__json_object *obj; >> + >> + obj = libxl__json_object_alloc(NOGC, JSON_STRING); >> + obj->u.string = libxl__strdup(NOGC, argument); >> + >> + qmp_parameters_common_add(gc, param, name, obj); >> +} >> + >> +static void qmp_parameters_add_bool(libxl__gc *gc, >> + libxl__json_object **param, >> + const char *name, bool b) >> +{ >> + libxl__json_object *obj; >> + >> + obj = libxl__json_object_alloc(NOGC, JSON_TRUE); > > This is pre-existing, but treating JSON_TRUE and JSON_FALSE as distinct > node types and not specific values of the (currently non-existent) > JSON_BOOL type is a bit odd. It was to save a bit of memory, and I probably follow another example when I wrote those JSON_{TRUE,FALSE}. I'll add another patch to introduce JSON_BOOL and remove those separated true/false. > I noticed it because you don't use the b param here... :(. >> + qmp_parameters_common_add(gc, param, name, obj); >> +} >> + >> +#define QMP_PARAMETERS_SPRINTF(gc, args, name, format, ...) \ >> + qmp_parameters_add_string(gc, args, name, \ >> + libxl__sprintf(gc, format, __VA_ARGS__)) > > I think it would be valid, and in keeping with the similar GC_SPRINTFOO > macros, to make gc and perhaps args required in the calling environment > rather than passing them, if you wanted to. Seams fair to have gc required in the caller but I think I'll keep args as a parameter of the macro. Thanks, -- Anthony PERARD