Re: [PATCH v2 02/10] libxl-pci: removing the extra 'out_closedir' cleaning path

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1789 bytes --]

On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote:
> Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
> 
> ---
> Changed since v1:
>  * Modify the libxl_device_pci_assignable_list() function to use
>    only one 'out' cleaning path.
>
So, the patch does quite a few style cleanups. This is indeed a good
thing, but both subject and changelog need to better reflect it.

I'd go for the following.

Subject: "libxl: style cleanups in libxl_device_pci_assignable_list()

Changelog:
"various coding style compliance cleanups, such as, arranging for 
 using only one path out of the function, whitespaces in loops ad if-s 
 and r instead of rc for storing non-libxl error codes."

Again, the code looks ok to me. Just one suggestion:

> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -395,34 +395,35 @@ libxl_device_pci
> *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
>      libxl_device_pci *pcidevs = NULL, *new, *assigned;
>      struct dirent *de;
>      DIR *dir;
> -    int rc, num_assigned;
> +    int r, num_assigned;
>  
>      *num = 0;
>  
> -    rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
> -    if ( rc )
> +    r = get_all_assigned_devices(gc, &assigned, &num_assigned);
> +    if (r)
>          goto out;
>  
libxl CODING_STYLE allows 'if (r) goto out;', which, since you're
refactoring, you may want to use here.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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