Re: [PATCH 2/3] interface: Introduce virInterfaceObjGetNames

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/06/2017 04:08 PM, John Ferlan wrote:
Unlike other drivers, this is a test driver only API. Still combining
the logic of testConnectListInterfaces and testConnectListDefinedInterfaces
makes things a bit easier in the long run.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/conf/virinterfaceobj.c | 34 +++++++++++++++++++++++++++++
 src/conf/virinterfaceobj.h |  6 ++++++
 src/libvirt_private.syms   |  1 +
 src/test/test_driver.c     | 54 +++++++++++-----------------------------------
 4 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 0407c1f..229226a 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -235,3 +235,37 @@ virInterfaceObjNumOfInterfaces(virInterfaceObjListPtr interfaces,

     return ninterfaces;
 }
+
+
+int
+virInterfaceObjGetNames(virInterfaceObjListPtr interfaces,
+                        bool wantActive,
+                        char **const names,
+                        int maxnames)
+{
+    int nnames = 0;
+    size_t i;
+
+    for (i = 0; i < interfaces->count && nnames < maxnames; i++) {
+        virInterfaceObjPtr obj = interfaces->objs[i];
+        virInterfaceObjLock(obj);
+        if ((wantActive && virInterfaceObjIsActive(obj)) ||
+            (!wantActive && !virInterfaceObjIsActive(obj))) {

Again. if (wantActive == virInterfaceObjIsActive()) ...

+            if (VIR_STRDUP(names[nnames], obj->def->name) < 0) {
+                virInterfaceObjUnlock(obj);
+                goto failure;
+            }
+            nnames++;
+        }
+        virInterfaceObjUnlock(obj);
+    }
+
+    return nnames;
+
+ failure:
+    while (--nnames >= 0)
+        VIR_FREE(names[nnames]);
+    memset(names, 0, maxnames * sizeof(*names));

This isn't necessary. Firstly, VIR_FREE() already sets all those items in the array we've touched to zero. Secondly, the callers already clear our the array (even those items we haven't touched). But mostly, in general, if an error is returned, callers should not rely on anything else that is returned. For instance, should virStrToLong_*(s, &ret, base, &result) return -1, the @result is undefined.

+
+    return -1;
+}
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index 2f07174..5b0527d 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -85,4 +85,10 @@ int
 virInterfaceObjNumOfInterfaces(virInterfaceObjListPtr interfaces,
                                bool wantActive);

+int
+virInterfaceObjGetNames(virInterfaceObjListPtr interfaces,
+                        bool wantActive,
+                        char **const names,
+                        int maxnames);
+
 #endif /* __VIRINTERFACEOBJ_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 96aacaa..88e530c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -935,6 +935,7 @@ virDomainObjListRename;
 virInterfaceObjAssignDef;
 virInterfaceObjFindByMACString;
 virInterfaceObjFindByName;
+virInterfaceObjGetNames;
 virInterfaceObjListClone;
 virInterfaceObjListFree;
 virInterfaceObjLock;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6910681..4e10eb2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3657,33 +3657,18 @@ static int testConnectNumOfInterfaces(virConnectPtr conn)
     return ninterfaces;
 }

-static int testConnectListInterfaces(virConnectPtr conn, char **const names, int nnames)
+static int testConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames)
 {
     testDriverPtr privconn = conn->privateData;
-    int n = 0;
-    size_t i;
+    int nnames;
+
+    memset(names, 0, maxnames * sizeof(*names));

I don't think this is necessary either, but I don't care that much.


     testDriverLock(privconn);
-    memset(names, 0, sizeof(*names)*nnames);
-    for (i = 0; (i < privconn->ifaces.count) && (n < nnames); i++) {
-        virInterfaceObjLock(privconn->ifaces.objs[i]);
-        if (virInterfaceObjIsActive(privconn->ifaces.objs[i])) {
-            if (VIR_STRDUP(names[n++], privconn->ifaces.objs[i]->def->name) < 0) {
-                virInterfaceObjUnlock(privconn->ifaces.objs[i]);
-                goto error;
-            }
-        }
-        virInterfaceObjUnlock(privconn->ifaces.objs[i]);
-    }
+    nnames = virInterfaceObjGetNames(&privconn->ifaces, true, names, maxnames);
     testDriverUnlock(privconn);

-    return n;
-
- error:
-    for (n = 0; n < nnames; n++)
-        VIR_FREE(names[n]);
-    testDriverUnlock(privconn);
-    return -1;
+    return nnames;

ACK

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux