Re: [libvirt] [PATCH] nwfilter: cleanup return codes in nwfilter subsystem

23 Nov 2011


      On 11/23/2011 02:19 PM, Stefan Berger wrote:
...
This patch cleans up return codes in the nwfilter subsystem.
Some functions in nwfilter_conf.c (validators and formatters) are
keeping their bool return for now and I am converting their return
code to true/false.
All other functions now return -1 on failure and 0 on success.
[I searched for all occurences of ' 1;' and checked all 'if ' and
adapted where needed. After that I did a grep for 'NWFilter' in the source
tree.]
---
I compile tested this, and assume you also ran it through libvirt-TCK.
My compile run complained:

qemu/qemu_command.c: In function 'qemuNetworkIfaceConnect':
qemu/qemu_command.c:278:65: error: suggest braces around empty body in
an 'if' statement [-Werror=empty-body]

[more on this below]
...
-static bool
+static int
 intMapGetByInt(const struct int_map *intmap, int32_t attr, const char **res)
 {
     int i = 0;
-    bool found = 0;
+    int found = false;
This should still be 'bool found', given how you assign bool values to it.
...
-static bool
+static int
 intMapGetByString(const struct int_map *intmap, const char *str, int casecmp,
                   int32_t *result)
Conversion of intMapGetBy{Int,String} is correct, including all callers.
 (I'm not sure if this would have been easier broken up into multiple
patches, a couple functions per patch, but don't bother splitting it now).
...
@@ -367,14 +369,14 @@ virNWFilterRuleDefAddVar(virNWFilterRule
if (VIR_REALLOC_N(nwf->vars, nwf->nvars+1) < 0) {
         virReportOOMError();
-        return 1;
+        return -1;
Conversion of virNWFilterRuleDefAddVar is correct, including all callers.
...
@@ -805,7 +808,7 @@ parseStringItems(const struct int_map *i
             }
         }
         if (!found) {
-            rc = 1;
+            rc = -1;
You still have two callers to parseStringItems that merely checked for
non-zero, rather than < 0 (in tcpFlagsValidator), but no logic
regression introduced.
...
@@ -1663,13 +1666,11 @@ static const virAttributes virAttr[] = {
 };
-static bool
+static int
 virNWMACAddressParser(const char *input,
                       nwMACAddressPtr output)
 {
-    if (virParseMacAddr(input, &output->addr[0]) == 0)
-        return 1;
-    return 0;
+    return virParseMacAddr(input, &output->addr[0]);
Conversion of virNWMACAddressParser is correct.
...
@@ -2649,7 +2650,7 @@ _virNWFilterDefLoopDetect(virConnectPtr 
         if (entry->include) {
if (STREQ(filtername, entry->include->filterref)) {
-                rc = 1;
+                rc = -1;
conversion of [_]virNWFilterDefLoopDetect and callers looks correct.
...
@@ -2736,7 +2737,7 @@ virNWFilterTriggerVMFilterRebuild(virCon
     };
if (!cb.skipInterfaces)
-        return 1;
+        return -1;
for (i = 0; i < nCallbackDriver; i++) {
         callbackDrvArray[i]->vmFilterRebuild(conn,
This one's tricky - you have err = cb.err, but I didn't check whether
all callers to virNWFilterConfLayerInit() pass a function that sets
cb.err to -1, or if you still need more cleanup.
...
@@ -2778,7 +2779,7 @@ virNWFilterTestUnassignDef(virConnectPtr
     nwfilter->wantRemoved = 1;
     /* trigger the update on VMs referencing the filter */
     if (virNWFilterTriggerVMFilterRebuild(conn))
-        rc = 1;
+        rc = -1;
Conversion of virNWFilterTestUnassignDef and callers looks correct.
...
@@ -3297,8 +3298,8 @@ int virNWFilterConfLayerInit(virHashIter
initialized = true;
-    if (virMutexInitRecursive(&updateMutex))
-        return 1;
+    if (virMutexInitRecursive(&updateMutex) < 0)
+        return -1;
Conversion of virNWFilterConfLayerInit() and callers looks correct.
...
return 0;
 }
Index: libvirt-acl/src/nwfilter/nwfilter_driver.c
This is as far as I got today.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

    