On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote: > Signed-off-by: Paulina Szubarczyk > > --- > 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 -- <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)