From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Jackson Subject: Re: [PATCH v4 2/8] libxl: libxl__device_disk_local_attach return a new libxl_device_disk Date: Tue, 24 Apr 2012 16:02:29 +0100 Message-ID: <20374.49285.902510.115655@mariner.uk.xensource.com> References: <1335264358-20182-2-git-send-email-stefano.stabellini@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1335264358-20182-2-git-send-email-stefano.stabellini@eu.citrix.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: Stefano Stabellini Cc: xen-devel@lists.xensource.com, Ian.Campbell@citrix.com List-Id: xen-devel@lists.xenproject.org Stefano Stabellini writes ("[Xen-devel] [PATCH v4 2/8] libxl: libxl__device_disk_local_attach return a new libxl_device_disk"): ... > +_hidden char * libxl__device_disk_local_attach(libxl__gc *gc, > + const libxl_device_disk *disk, > + libxl_device_disk **new_disk) > { > libxl_ctx *ctx = gc->owner; > char *dev = NULL; > - char *ret = NULL; > int rc; > - > - rc = libxl__device_disk_setdefault(gc, disk); > + libxl_device_disk *tmpdisk = libxl__zalloc(gc, sizeof(libxl_device_disk)); > + if (tmpdisk == NULL) goto out; libxl__zalloc is guaranteed not to fail, nowadays. > - switch (disk->backend) { > + switch (tmpdisk->backend) { Did you see my comment about arranging for "disk" to be the new structure and renaming the formal parameter ? That would (a) remove a bunch of useless stuff from the diff (b) probably make the code clearer anyway, since nothing inside this function should be using the actually-passed libxl_device_disk* other than the code whose purpose is to make a copy of it. If you resend with that change I will find it much easier to review this. Ian.