Re: [Xen-devel] [Patch v3 3/4] tools/libxl: Fix libxl__device_nic_from_xs_be()

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Andrew Cooper writes ("Re: [Xen-devel] [Patch v3 3/4] tools/libxl: Fix 
libxl__device_nic_from_xs_be()"):
> As this function cant fail, I was trying to force all error paths to
> apply safe defaults to the libxl_device_nic structure.

Perhaps the function should be able to fail.

From 3cea493c97f23eeb8e175915186f7ca2701da60a Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date: Tue, 26 Nov 2013 12:08:09 +0000
Subject: [PATCH] libxl: Fix error handling in libxl__device_nic_from_xs_be

This requires changing its return type and fixing the callers.

Introduce here a READ_BACKEND macro to make the code less repetitive.

Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
 tools/libxl/libxl.c |   62 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2b847ef..62ff6db 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2976,45 +2976,51 @@ out:
     return;
 }
 
-static void libxl__device_nic_from_xs_be(libxl__gc *gc,
-                                         const char *be_path,
-                                         libxl_device_nic *nic)
+static int libxl__device_nic_from_xs_be(libxl__gc *gc,
+                                        const char *be_path,
+                                        libxl_device_nic *nic)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    unsigned int len;
-    char *tmp;
+    const char *tmp;
     int rc;
 
     libxl_device_nic_init(nic);
 
-    tmp = xs_read(ctx->xsh, XBT_NULL,
-                  libxl__sprintf(gc, "%s/handle", be_path), &len);
-    if ( tmp )
+#define READ_BACKEND(subpath) ({                                        \
+        rc = libxl__xs_read_checked(gc, XBT_NULL,                       \
+                                    GCSPRINTF("%s/" subpath, be_path),  \
+                                    &tmp);                              \
+        if (rc) goto out;                                               \
+        (char*)tmp;                                                     \
+    });
+
+    tmp = READ_BACKEND("handle");
+    if (tmp)
         nic->devid = atoi(tmp);
     else
         nic->devid = 0;
 
     /* nic->mtu = */
 
-    tmp = xs_read(ctx->xsh, XBT_NULL,
-                  libxl__sprintf(gc, "%s/mac", be_path), &len);
-    rc = libxl__parse_mac(tmp, nic->mac);
-    if (rc)
+    tmp = READ_BACKEND("mac");
+    if (tmp) {
+        rc = libxl__parse_mac(tmp, nic->mac);
+        if (rc) goto out;
+    } else {
         memset(nic->mac, 0, sizeof(nic->mac));
+    }
 
-    nic->ip = xs_read(ctx->xsh, XBT_NULL,
-                      libxl__sprintf(gc, "%s/ip", be_path), &len);
-
-    nic->bridge = xs_read(ctx->xsh, XBT_NULL,
-                      libxl__sprintf(gc, "%s/bridge", be_path), &len);
-
-    nic->script = xs_read(ctx->xsh, XBT_NULL,
-                      libxl__sprintf(gc, "%s/script", be_path), &len);
+    nic->ip = READ_BACKEND("ip");
+    nic->bridge = READ_BACKEND("bridge");
+    nic->script = READ_BACKEND("script");
 
     /* vif_ioemu nics use the same xenstore entries as vif interfaces */
     nic->nictype = LIBXL_NIC_TYPE_VIF;
     nic->model = NULL; /* XXX Only for TYPE_IOEMU */
     nic->ifname = NULL; /* XXX Only for TYPE_IOEMU */
+
+    rc = 0;
+ out:
+    return rc;
 }
 
 int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
@@ -3035,7 +3041,8 @@ int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t 
domid,
     if (!path)
         goto out;
 
-    libxl__device_nic_from_xs_be(gc, path, nic);
+    rc = libxl__device_nic_from_xs_be(gc, path, nic);
+    if (rc) goto out;
 
     rc = 0;
 out:
@@ -3053,6 +3060,7 @@ static int libxl__append_nic_list_of_type(libxl__gc *gc,
     char **dir = NULL;
     unsigned int n = 0;
     libxl_device_nic *pnic = NULL, *pnic_end = NULL;
+    int rc;
 
     be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
                              libxl__xs_get_dompath(gc, 0), type, domid);
@@ -3064,16 +3072,20 @@ static int libxl__append_nic_list_of_type(libxl__gc *gc,
             return ERROR_NOMEM;
         *nics = tmp;
         pnic = *nics + *nnics;
-        *nnics += n;
-        pnic_end = *nics + *nnics;
+        pnic_end = *nics + *nnics + n;
         for (; pnic < pnic_end; pnic++, dir++) {
             const char *p;
             p = libxl__sprintf(gc, "%s/%s", be_path, *dir);
-            libxl__device_nic_from_xs_be(gc, p, pnic);
+            rc = libxl__device_nic_from_xs_be(gc, p, pnic);
+            if (rc) goto out;
             pnic->backend_domid = 0;
         }
+        *nnics += n;
     }
     return 0;
+
+ out:
+    return rc;
 }
 
 libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int 
*num)
-- 
1.7.10.4


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