[libvirt] [PATCH v2] nwfilter: Fix return value of virNWFilterHashTablePut

[Michal Privoznik]

In libvirt it is common to return value <0 on error not vice versa.
---
diff to v1:
-check 3rd call as well:
https://www.redhat.com/archives/libvir-list/2011-November/msg01312.html

 src/conf/nwfilter_params.c             |   16 +++++++-------
 src/nwfilter/nwfilter_gentech_driver.c |   37 ++++++++++++++-----------------
 src/nwfilter/nwfilter_learnipaddr.c    |    5 +---
 3 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index 12c6524..ba41972 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -490,7 +490,7 @@ hashDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
  * @val: The value associated with the key
  * @freeName: Whether the name must be freed on table destruction
  *
- * Returns 0 on success, 1 on failure.
+ * Returns 0 on success, -1 on failure.
  *
  * Put an entry into the hashmap replacing and freeing an existing entry
  * if one existed.
@@ -505,11 +505,11 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table,
         if (copyName) {
             name = strdup(name);
             if (!name)
-                return 1;
+                return -1;
 
             if (VIR_REALLOC_N(table->names, table->nNames + 1) < 0) {
                 VIR_FREE(name);
-                return 1;
+                return -1;
             }
             table->names[table->nNames++] = (char *)name;
         }
@@ -519,11 +519,11 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table,
                 VIR_FREE(name);
                 table->nNames--;
             }
-            return 1;
+            return -1;
         }
     } else {
         if (virHashUpdateEntry(table->hashTable, name, val) != 0) {
-            return 1;
+            return -1;
         }
     }
     return 0;
@@ -614,7 +614,7 @@ addToTable(void *payload, const void *name, void *data)
         return;
     }
 
-    if (virNWFilterHashTablePut(atts->target, (const char *)name, val, 1) != 0) {
+    if (virNWFilterHashTablePut(atts->target, (const char *)name, val, 1) < 0) {
         virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Could not put variable '%s' into hashmap"),
                                (const char *)name);
@@ -640,7 +640,7 @@ virNWFilterHashTablePutAll(virNWFilterHashTablePtr src,
     return 0;
 
 err_exit:
-    return 1;
+    return -1;
 }
 
 
@@ -700,7 +700,7 @@ virNWFilterParseParamAttributes(xmlNodePtr cur)
                         value = virNWFilterParseVarValue(val);
                         if (!value)
                             goto skip_entry;
-                        if (virNWFilterHashTablePut(table, nam, value, 1))
+                        if (virNWFilterHashTablePut(table, nam, value, 1) < 0)
                             goto err_exit;
                     }
                     value = NULL;
diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 84a959b..a0cb05c 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -343,10 +343,10 @@ virNWFilterCreateVarsFrom(virNWFilterHashTablePtr vars1,
         return NULL;
     }
 
-    if (virNWFilterHashTablePutAll(vars1, res))
+    if (virNWFilterHashTablePutAll(vars1, res) < 0)
         goto err_exit;
 
-    if (virNWFilterHashTablePutAll(vars2, res))
+    if (virNWFilterHashTablePutAll(vars2, res) < 0)
         goto err_exit;
 
     return res;
@@ -499,7 +499,7 @@ virNWFilterDetermineMissingVarsRec(virConnectPtr conn,
                                    virNWFilterDriverStatePtr driver)
 {
     virNWFilterObjPtr obj;
-    int rc = 0;
+    int rc = -1;
     int i, j;
     virNWFilterDefPtr next_filter;
     virNWFilterVarValuePtr val;
@@ -512,16 +512,12 @@ virNWFilterDetermineMissingVarsRec(virConnectPtr conn,
             for (j = 0; j < rule->nvars; j++) {
                 if (!virHashLookup(vars->hashTable, rule->vars[j])) {
                     val = virNWFilterVarValueCreateSimpleCopyValue("1");
-                    if (!val) {
-                        rc = 1;
-                        break;
-                    }
-                    virNWFilterHashTablePut(missing_vars, rule->vars[j],
-                                            val, 1);
+                    if (!val ||
+                        virNWFilterHashTablePut(missing_vars, rule->vars[j],
+                                                val, 1) < 0)
+                        goto cleanup;
                 }
             }
-            if (rc)
-                break;
         } else if (inc) {
             VIR_DEBUG("Following filter %s\n", inc->filterref);
             obj = virNWFilterObjFindByName(&driver->nwfilters, inc->filterref);
@@ -531,9 +527,8 @@ virNWFilterDetermineMissingVarsRec(virConnectPtr conn,
                     virNWFilterReportError(VIR_ERR_NO_NWFILTER,
                                            _("Filter '%s' is in use."),
                                            inc->filterref);
-                    rc = 1;
                     virNWFilterObjUnlock(obj);
-                    break;
+                    goto cleanup;
                 }
 
                 /* create a temporary hashmap for depth-first tree traversal */
@@ -542,9 +537,8 @@ virNWFilterDetermineMissingVarsRec(virConnectPtr conn,
                                                                 vars);
                 if (!tmpvars) {
                     virReportOOMError();
-                    rc = 1;
                     virNWFilterObjUnlock(obj);
-                    break;
+                    goto cleanup;
                 }
 
                 next_filter = obj->def;
@@ -569,17 +563,20 @@ virNWFilterDetermineMissingVarsRec(virConnectPtr conn,
                 virNWFilterHashTableFree(tmpvars);
 
                 virNWFilterObjUnlock(obj);
-                if (rc)
-                    break;
+                if (rc < 0)
+                    goto cleanup;
             } else {
                 virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("referenced filter '%s' is missing"),
                                        inc->filterref);
-                rc = 1;
-                break;
+                goto cleanup;
             }
         }
     }
+
+    rc = 0;
+
+cleanup:
     return rc;
 }
 
@@ -671,7 +668,7 @@ virNWFilterInstantiate(virConnectPtr conn,
                                             missing_vars,
                                             useNewFilter,
                                             driver);
-    if (rc)
+    if (rc < 0)
         goto err_exit;
 
     if (virHashSize(missing_vars->hashTable) == 1) {
diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index 6f2cc4c..425e8a2 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -336,9 +336,6 @@ virNWFilterAddIpAddrForIfname(const char *ifname, char *addr)
             goto cleanup;
         }
         ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1);
-        /* FIXME: fix when return code of virNWFilterHashTablePut changes */
-        if (ret)
-            ret = -1;
         goto cleanup;
     } else {
         if (virNWFilterVarValueAddValue(val, addr) < 0)
@@ -803,7 +800,7 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver,
         goto err_free_req;
     }
 
-    if (virNWFilterHashTablePutAll(filterparams, ht))
+    if (virNWFilterHashTablePutAll(filterparams, ht) < 0)
         goto err_free_ht;
 
     req->filtername = strdup(filtername);
-- 
1.7.3.4