| Message ID | 1459943163-18697-3-git-send-email-paulinaszubarczyk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index 9c9cd04..8549378 100644 --- 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; dir = opendir(SYSFS_PCIBACK_DRIVER); - if ( NULL == dir ) { - if ( errno == ENOENT ) { + if (NULL == dir) { + if (errno == ENOENT) { LOG(ERROR, "Looks like pciback driver not loaded"); - }else{ + } else { LOGE(ERROR, "Couldn't open %s", SYSFS_PCIBACK_DRIVER); } - goto out_closedir; + closedir(dir); + goto out; } - while( (de = readdir(dir)) ) { + while((de = readdir(dir))) { unsigned dom, bus, dev, func; - if ( sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4 ) + if (sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4) continue; - if ( is_pcidev_in_array(assigned, num_assigned, dom, bus, dev, func) ) + if (is_pcidev_in_array(assigned, num_assigned, dom, bus, dev, func)) continue; new = realloc(pcidevs, ((*num) + 1) * sizeof(*new)); - if ( NULL == new ) + if (NULL == new) continue; pcidevs = new; @@ -433,8 +434,6 @@ libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num) (*num)++; } -out_closedir: - closedir(dir); out: GC_FREE; return pcidevs;
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. --- tools/libxl/libxl_pci.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)