Re: [Xen-devel] [PATCH v4 2/8] libxl: libxl__device_disk_local_attach return a new libxl_device_disk

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel