[v3,2/7] libxl: style cleanups in libxl_device_pci_assignable_list()

Commit Message

Paulina Szubarczyk April 20, 2016, 8:03 a.m. UTC

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.

Signed-off-by: Paulina Szubarczyk <[e-mail]>
---
Changes since v2:
 - changed the changelog
---
 tools/libxl/libxl_pci.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

Comments

Olaf Hering April 20, 2016, 8:24 p.m. UTC | #1

On Wed, Apr 20, Paulina Szubarczyk wrote:

> @@ -398,34 +398,34 @@ libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
>      dir = opendir(SYSFS_PCIBACK_DRIVER);

> +    while((de = readdir(dir))) {

> -    closedir(dir);

This change seems to leak 'dir' in success case.

Olaf

Paulina Szubarczyk April 21, 2016, 7:17 a.m. UTC | #2

On Wed, 2016-04-20 at 22:24 +0200, Olaf Hering wrote:
> On Wed, Apr 20, Paulina Szubarczyk wrote:
> 
> > @@ -398,34 +398,34 @@ libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
> >      dir = opendir(SYSFS_PCIBACK_DRIVER);
> 
> > +    while((de = readdir(dir))) {
> 
> > -    closedir(dir);
> 
> This change seems to leak 'dir' in success case.

I am sorry, that is true. 

The previous out_closedir path was run from the path after the
condition:
+    if (NULL == dir) {

-        goto out_closedir;
+        closedir(dir);
+        goto out;
     }

-out_closedir:
-    closedir(dir);
 out:
     GC_FREE;
     return pcidevs;

so in this case it could just jump to goto out without calling
closedir(NULL). 

Paulina

> Olaf

Patch

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index b4be967..6ac0a92 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -398,34 +398,34 @@  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 )
-        goto out;
+    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;
@@ -436,8 +436,6 @@  libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
         (*num)++;
     }
 
-out_closedir:
-    closedir(dir);
 out:
     GC_FREE;
     return pcidevs;