From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony PERARD Subject: Re: [PATCH V2 1/9] libxl_json: Use libxl alloc function with NOGC. Date: Tue, 25 Sep 2012 14:54:52 +0100 Message-ID: <5061B7AC.508@citrix.com> References: <1347906148-9606-1-git-send-email-anthony.perard@citrix.com> <1347906148-9606-2-git-send-email-anthony.perard@citrix.com> <1348562359.3452.112.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: <1348562359.3452.112.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: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 >> --- >> 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