Re: [PATCH V2 1/9] libxl_json: Use libxl alloc function with NOGC.

[Anthony PERARD <anthony.perard@citrix.com>]

On 09/25/2012 09:39 AM, Ian Campbell wrote:
> On Mon, 2012-09-17 at 19:22 +0100, Anthony PERARD wrote:
>> This patch makes use of the libxl allocation API and removes the check for
>> allocation failure.
>>
>> Also we don't use GC with a json_object because this structure use flexarray
>> and the latter does not use the gc.
> 
> It's not an uncommon pattern in libxl for the content of the flexarray
> to be gc'd but the actual array itself to be explicitly freed, often
> implicitly via flexarray_contents(), if that's what you want.

Trying to use flexarray_contents() in the context of json_object would
mean that I need to know when the array is filled with every things
needed. This seams a bit complicated.

> If we wanted I don't think there's any reason we couldn't make the
> flexarray take a gc and use it, that would probably make things simpler
> here and elsewhere and reduce the manual memory management (unless you
> actually want/need that for some other reason).

Having flexarray using gc seams a better idea. I will work on that as I
don't think I need to keep those object around and using NOGC was only
because of flexarray.

>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>  tools/libxl/libxl_json.c | 37 +++++++++++--------------------------
>>  1 file changed, 11 insertions(+), 26 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
>> index caa8312..9c3dca2 100644
>> --- a/tools/libxl/libxl_json.c
>> +++ b/tools/libxl/libxl_json.c
>> @@ -210,12 +210,7 @@ static libxl__json_object *json_object_alloc(libxl__gc *gc,
>>  {
>>      libxl__json_object *obj;
>>  
>> -    obj = calloc(1, sizeof (libxl__json_object));
>> -    if (obj == NULL) {
>> -        LIBXL__LOG_ERRNO(libxl__gc_owner(gc), LIBXL__LOG_ERROR,
>> -                         "Failed to allocate a libxl__json_object");
>> -        return NULL;
>> -    }
>> +    obj = libxl__zalloc(NOGC, sizeof(*obj));
> 
> So you now ignore the gc passed in, which in any case you have now
> caused to always be NOGC? Seems a bit round-about to me, why not use the
> gc parameter here?

Yes, I suppose is not necessary to use NOGC here, and leave the choice
to the caller. I just wanted to be explicit.

Thanks,

-- 
Anthony PERARD