nwfilter: cleanup return codes in nwfilter subsystem

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 have failure return codes of -1 and success
of 0.

[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.]

src/conf/nwfilter_conf.c

@@ -214,23 +214,24 @@ static const char state_str[]        = "state";
  * @attr: The attribute to look up
  * @res: Pointer to string pointer for result
  *
- * Returns 1 if value was found with result returned, 0 otherwise.
+ * Returns 0 if value was found with result returned, -1 otherwise.
  *
  * lookup a map entry given the integer.
  */
-static bool
+static int
 intMapGetByInt(const struct int_map *intmap, int32_t attr, const char **res)
 {
     int i = 0;
-    bool found = 0;
+    bool found = false;
+
     while (intmap[i].val && !found) {
         if (intmap[i].attr == attr) {
             *res = intmap[i].val;
-            found = 1;
+            found = true;
         }
         i++;
     }
-    return found;
+    return (found) ? 0 : -1;
 }
 
 
@@ -241,26 +242,27 @@ intMapGetByInt(const struct int_map *intmap, int32_t attr, const char **res)
  * @casecmp : Whether to ignore case when doing string matching
  * @result: Pointer to int for result
  *
- * Returns 0 if no entry was found, 1 otherwise.
+ * Returns 0 if entry was found, -1 otherwise.
  *
  * Do a lookup in the map trying to find an integer key using the string
- * value. Returns 1 if entry was found with result returned, 0 otherwise.
+ * value. Returns 0 if entry was found with result returned, -1 otherwise.
  */
-static bool
+static int
 intMapGetByString(const struct int_map *intmap, const char *str, int casecmp,
                   int32_t *result)
 {
     int i = 0;
-    bool found = 0;
+    bool found = false;
+
     while (intmap[i].val && !found) {
         if ( (casecmp && STRCASEEQ(intmap[i].val, str)) ||
                          STREQ    (intmap[i].val, str)    ) {
             *result = intmap[i].attr;
-            found = 1;
+            found = true;
         }
         i++;
     }
-    return found;
+    return (found) ? 0 : -1;
 }
 
 
@@ -367,14 +369,14 @@ virNWFilterRuleDefAddVar(virNWFilterRuleDefPtr nwf,
 
     if (VIR_REALLOC_N(nwf->vars, nwf->nvars+1) < 0) {
         virReportOOMError();
-        return 1;
+        return -1;
     }
 
     nwf->vars[nwf->nvars] = strdup(var);
 
     if (!nwf->vars[nwf->nvars]) {
         virReportOOMError();
-        return 1;
+        return -1;
     }
 
     item->var = nwf->vars[nwf->nvars++];
@@ -479,7 +481,7 @@ checkMacProtocolID(enum attrDatatype datatype, union data *value,
     int32_t res = -1;
 
     if (datatype == DATATYPE_STRING) {
-        if (intMapGetByString(macProtoMap, value->c, 1, &res) == 0)
+        if (intMapGetByString(macProtoMap, value->c, 1, &res) < 0)
             res = -1;
         datatype = DATATYPE_UINT16;
     } else if (datatype == DATATYPE_UINT16 ||
@@ -492,10 +494,10 @@ checkMacProtocolID(enum attrDatatype datatype, union data *value,
     if (res != -1) {
         nwf->p.ethHdrFilter.dataProtocolID.u.u16 = res;
         nwf->p.ethHdrFilter.dataProtocolID.datatype = datatype;
-        return 1;
+        return true;
     }
 
-    return 0;
+    return false;
 }
 
 
@@ -509,7 +511,7 @@ macProtocolIDFormatter(virBufferPtr buf,
 
     if (intMapGetByInt(macProtoMap,
                        nwf->p.ethHdrFilter.dataProtocolID.u.u16,
-                       &str)) {
+                       &str) == 0) {
         virBufferAdd(buf, str, -1);
     } else {
         if (nwf->p.ethHdrFilter.dataProtocolID.datatype == DATATYPE_UINT16)
@@ -517,7 +519,7 @@ macProtocolIDFormatter(virBufferPtr buf,
         virBufferAsprintf(buf, asHex ? "0x%x" : "%d",
                           nwf->p.ethHdrFilter.dataProtocolID.u.u16);
     }
-    return 1;
+    return true;
 }
 
 
@@ -550,7 +552,7 @@ checkVlanProtocolID(enum attrDatatype datatype, union data *value,
     int32_t res = -1;
 
     if (datatype == DATATYPE_STRING) {
-        if (intMapGetByString(macProtoMap, value->c, 1, &res) == 0)
+        if (intMapGetByString(macProtoMap, value->c, 1, &res) < 0)
             res = -1;
         datatype = DATATYPE_UINT16;
     } else if (datatype == DATATYPE_UINT16 ||
@@ -579,7 +581,7 @@ vlanProtocolIDFormatter(virBufferPtr buf,
 
     if (intMapGetByInt(macProtoMap,
                        nwf->p.vlanHdrFilter.dataVlanEncap.u.u16,
-                       &str)) {
+                       &str) == 0) {
         virBufferAdd(buf, str, -1);
     } else {
         if (nwf->p.vlanHdrFilter.dataVlanEncap.datatype == DATATYPE_UINT16)
@@ -607,7 +609,7 @@ checkValidMask(unsigned char *data, int len)
                 checkones = 0;
         } else {
             if ((data[idx>>3] & mask))
-                return 0;
+                return false;
         }
 
         idx++;
@@ -615,7 +617,7 @@ checkValidMask(unsigned char *data, int len)
         if (!mask)
             mask = 0x80;
     }
-    return 1;
+    return true;
 }
 
 
@@ -655,7 +657,7 @@ arpOpcodeValidator(enum attrDatatype datatype,
     int32_t res = -1;
 
     if (datatype == DATATYPE_STRING) {
-        if (intMapGetByString(arpOpcodeMap, value->c, 1, &res) == 0)
+        if (intMapGetByString(arpOpcodeMap, value->c, 1, &res) < 0)
             res = -1;
         datatype = DATATYPE_UINT16;
     } else if (datatype == DATATYPE_UINT16 ||
@@ -666,9 +668,9 @@ arpOpcodeValidator(enum attrDatatype datatype,
     if (res != -1) {
         nwf->p.arpHdrFilter.dataOpcode.u.u16 = res;
         nwf->p.arpHdrFilter.dataOpcode.datatype = datatype;
-        return 1;
+        return true;
     }
-    return 0;
+    return false;
 }
 
 
@@ -681,12 +683,12 @@ arpOpcodeFormatter(virBufferPtr buf,
 
     if (intMapGetByInt(arpOpcodeMap,
                        nwf->p.arpHdrFilter.dataOpcode.u.u16,
-                       &str)) {
+                       &str) == 0) {
         virBufferAdd(buf, str, -1);
     } else {
         virBufferAsprintf(buf, "%d", nwf->p.arpHdrFilter.dataOpcode.u.u16);
     }
-    return 1;
+    return true;
 }
 
 
@@ -708,15 +710,16 @@ static const struct int_map ipProtoMap[] = {
 };
 
 
-static bool checkIPProtocolID(enum attrDatatype datatype,
-                              union data *value,
-                              virNWFilterRuleDefPtr nwf,
-                              nwItemDesc *item ATTRIBUTE_UNUSED)
+static bool
+checkIPProtocolID(enum attrDatatype datatype,
+                  union data *value,
+                  virNWFilterRuleDefPtr nwf,
+                  nwItemDesc *item ATTRIBUTE_UNUSED)
 {
     int32_t res = -1;
 
     if (datatype == DATATYPE_STRING) {
-        if (intMapGetByString(ipProtoMap, value->c, 1, &res) == 0)
+        if (intMapGetByString(ipProtoMap, value->c, 1, &res) < 0)
             res = -1;
         datatype = DATATYPE_UINT8_HEX;
     } else if (datatype == DATATYPE_UINT8 ||
@@ -727,9 +730,9 @@ static bool checkIPProtocolID(enum attrDatatype datatype,
     if (res != -1) {
         nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8 = res;
         nwf->p.ipHdrFilter.ipHdr.dataProtocolID.datatype = datatype;
-        return 1;
+        return true;
     }
-    return 0;
+    return false;
 }
 
 
@@ -743,7 +746,7 @@ formatIPProtocolID(virBufferPtr buf,
 
     if (intMapGetByInt(ipProtoMap,
                        nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8,
-                       &str)) {
+                       &str) == 0) {
         virBufferAdd(buf, str, -1);
     } else {
         if (nwf->p.ipHdrFilter.ipHdr.dataProtocolID.datatype == DATATYPE_UINT8)
@@ -751,7 +754,7 @@ formatIPProtocolID(virBufferPtr buf,
         virBufferAsprintf(buf, asHex ? "0x%x" : "%d",
                           nwf->p.ipHdrFilter.ipHdr.dataProtocolID.u.u8);
     }
-    return 1;
+    return true;
 }
 
 
@@ -762,11 +765,11 @@ dscpValidator(enum attrDatatype datatype, union data *val,
 {
     uint8_t dscp = val->ui;
     if (dscp > 63)
-        return 0;
+        return false;
 
     nwf->p.ipHdrFilter.ipHdr.dataDSCP.datatype = datatype;
 
-    return 1;
+    return true;
 }
 
 
@@ -805,7 +808,7 @@ parseStringItems(const struct int_map *int_map,
             }
         }
         if (!found) {
-            rc = 1;
+            rc = -1;
             break;
         }
     }
@@ -874,15 +877,15 @@ stateValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, union data *val,
     char *input = val->c;
     int32_t flags = 0;
 
-    if (parseStateMatch(input, &flags))
-        return 0;
+    if (parseStateMatch(input, &flags) < 0)
+        return false;
 
     item->u.u16 = flags;
     nwf->flags |= flags;
 
     item->datatype = DATATYPE_UINT16;
 
-    return 1;
+    return true;
 }
 
 
@@ -929,8 +932,8 @@ tcpFlagsValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, union data *val,
 
     *sep = '\0';
 
-    if (!parseStringItems(tcpFlags, s_mask , &mask , ',') &&
-        !parseStringItems(tcpFlags, s_flags, &flags, ',')) {
+    if (parseStringItems(tcpFlags, s_mask , &mask , ',') == 0 &&
+        parseStringItems(tcpFlags, s_flags, &flags, ',') == 0 ) {
         item->u.tcpFlags.mask  = mask  & 0x3f;
         item->u.tcpFlags.flags = flags & 0x3f;
         rc = true;
@@ -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]);
 }
 
 
@@ -1714,7 +1715,7 @@ virNWFilterRuleDetailsParse(xmlNodePtr node,
                 flags_set |= NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR;
                 if (virNWFilterRuleDefAddVar(nwf,
                                              item,
-                                             &prop[1]))
+                                             &prop[1]) < 0)
                     rc = -1;
                 found = 1;
             }
@@ -1805,8 +1806,8 @@ virNWFilterRuleDetailsParse(xmlNodePtr node,
                         break;
 
                         case DATATYPE_MACADDR:
-                            if (!virNWMACAddressParser(prop,
-                                                       &item->u.macaddr)) {
+                            if (virNWMACAddressParser(prop,
+                                                      &item->u.macaddr) < 0) {
                                 rc = -1;
                             }
                             found = 1;
@@ -1814,8 +1815,8 @@ virNWFilterRuleDetailsParse(xmlNodePtr node,
 
                         case DATATYPE_MACMASK:
                             validator = checkMACMask;
-                            if (!virNWMACAddressParser(prop,
-                                                       &item->u.macaddr)) {
+                            if (virNWMACAddressParser(prop,
+                                                      &item->u.macaddr) < 0) {
                                 rc = -1;
                             }
                             data.v = &item->u.macaddr;
@@ -2418,8 +2419,8 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) {
         } else {
             /* assign default priority if none can be found via lookup */
             if (!name_prefix ||
-                !intMapGetByString(chain_priorities, name_prefix, 0,
-                                   &ret->chainPriority)) {
+                 intMapGetByString(chain_priorities, name_prefix, 0,
+                                   &ret->chainPriority) < 0) {
                 /* assign default chain priority */
                 ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
                                       NWFILTER_MIN_FILTER_PRIORITY) / 2;
@@ -2620,7 +2621,7 @@ int virNWFilterSaveConfig(const char *configDir,
     if (!(xml = virNWFilterDefFormat(def)))
         goto cleanup;
 
-    if (virNWFilterSaveXML(configDir, def, xml))
+    if (virNWFilterSaveXML(configDir, def, xml) < 0)
         goto cleanup;
 
     ret = 0;
@@ -2649,7 +2650,7 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
         if (entry->include) {
 
             if (STREQ(filtername, entry->include->filterref)) {
-                rc = 1;
+                rc = -1;
                 break;
             }
 
@@ -2660,8 +2661,8 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
                                                obj->def, filtername);
 
                 virNWFilterObjUnlock(obj);
-                if (rc)
-                   break;
+                if (rc < 0)
+                    break;
             }
         }
     }
@@ -2679,7 +2680,7 @@ _virNWFilterDefLoopDetect(virConnectPtr conn,
  * Detect a loop introduced through the filters being able to
  * reference each other.
  *
- * Returns 0 in case no loop was detected, 1 otherwise.
+ * Returns 0 in case no loop was detected, -1 otherwise.
  */
 static int
 virNWFilterDefLoopDetect(virConnectPtr conn,
@@ -2736,7 +2737,7 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
     };
 
     if (!cb.skipInterfaces)
-        return 1;
+        return -1;
 
     for (i = 0; i < nCallbackDriver; i++) {
         callbackDrvArray[i]->vmFilterRebuild(conn,
@@ -2778,7 +2779,7 @@ virNWFilterTestUnassignDef(virConnectPtr conn,
     nwfilter->wantRemoved = 1;
     /* trigger the update on VMs referencing the filter */
     if (virNWFilterTriggerVMFilterRebuild(conn))
-        rc = 1;
+        rc = -1;
 
     nwfilter->wantRemoved = 0;
 
@@ -2807,7 +2808,7 @@ virNWFilterObjAssignDef(virConnectPtr conn,
         virNWFilterObjUnlock(nwfilter);
     }
 
-    if (virNWFilterDefLoopDetect(conn, nwfilters, def)) {
+    if (virNWFilterDefLoopDetect(conn, nwfilters, def) < 0) {
         virNWFilterReportError(VIR_ERR_OPERATION_FAILED,
                               "%s", _("filter would introduce a loop"));
         return NULL;
@@ -3297,8 +3298,8 @@ int virNWFilterConfLayerInit(virHashIterator domUpdateCB)
 
     initialized = true;
 
-    if (virMutexInitRecursive(&updateMutex))
-        return 1;
+    if (virMutexInitRecursive(&updateMutex) < 0)
+        return -1;
 
     return 0;
 }

src/conf/nwfilter_params.c

@@ -82,7 +82,7 @@ virNWFilterVarValueCopy(const virNWFilterVarValuePtr val)
         }
         break;
     case NWFILTER_VALUE_TYPE_ARRAY:
-        if (VIR_ALLOC_N(res->u.array.values, val->u.array.nValues))
+        if (VIR_ALLOC_N(res->u.array.values, val->u.array.nValues) < 0)
             goto err_exit;
         res->u.array.nValues = val->u.array.nValues;
         for (i = 0; i < val->u.array.nValues; i++) {
@@ -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.
@@ -504,26 +504,28 @@ virNWFilterHashTablePut(virNWFilterHashTablePtr table,
     if (!virHashLookup(table->hashTable, name)) {
         if (copyName) {
             name = strdup(name);
-            if (!name)
-                return 1;
+            if (!name) {
+                virReportOOMError();
+                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;
         }
 
-        if (virHashAddEntry(table->hashTable, name, val) != 0) {
+        if (virHashAddEntry(table->hashTable, name, val) < 0) {
             if (copyName) {
                 VIR_FREE(name);
                 table->nNames--;
             }
-            return 1;
+            return -1;
         }
     } else {
-        if (virHashUpdateEntry(table->hashTable, name, val) != 0) {
-            return 1;
+        if (virHashUpdateEntry(table->hashTable, name, val) < 0) {
+            return -1;
         }
     }
     return 0;
@@ -614,7 +616,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 +642,7 @@ virNWFilterHashTablePutAll(virNWFilterHashTablePtr src,
     return 0;
 
 err_exit:
-    return 1;
+    return -1;
 }
 
 
@@ -700,7 +702,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;

src/nwfilter/nwfilter_driver.c

@@ -384,7 +384,7 @@ nwfilterUndefine(virNWFilterPtr obj) {
         goto cleanup;
     }
 
-    if (virNWFilterTestUnassignDef(obj->conn, nwfilter)) {
+    if (virNWFilterTestUnassignDef(obj->conn, nwfilter) < 0) {
         virNWFilterReportError(VIR_ERR_OPERATION_INVALID,
                                "%s",
                                _("nwfilter is in use"));

src/nwfilter/nwfilter_gentech_driver.c

@@ -98,7 +98,7 @@ virNWFilterTechDriverForName(const char *name) {
  * for bidirectional traffic and data needs to be added to the incoming
  * and outgoing chains.
  *
- * Returns 0 in case of success, 1 in case of an error.
+ * Returns 0 in case of success, -1 in case of an error.
  */
 int
 virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res,
@@ -106,7 +106,7 @@ virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res,
 {
     if (VIR_REALLOC_N(res->data, res->ndata+1) < 0) {
         virReportOOMError();
-        return 1;
+        return -1;
     }
     res->data[res->ndata++] = data;
     return 0;
@@ -136,7 +136,7 @@ virNWFilterRuleInstFree(virNWFilterRuleInstPtr inst)
  * @ipaddr: The string of the IP address to add to the hash table;
  *    may be NULL
  *
- * Returns 0 in case of success, 1 in case an error happened with
+ * Returns 0 in case of success, -1 in case an error happened with
  * error having been reported.
  *
  * Adds a couple of standard keys (MAC, IP) to the hash table.
@@ -151,28 +151,28 @@ virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table,
     if (macaddr) {
         val = virNWFilterVarValueCreateSimple(macaddr);
         if (!val)
-            return 1;
+            return -1;
 
         if (virHashAddEntry(table->hashTable,
                             NWFILTER_STD_VAR_MAC,
                             val) < 0) {
             virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
                                    "%s", _("Could not add variable 'MAC' to hashmap"));
-            return 1;
+            return -1;
         }
     }
 
     if (ipaddr) {
         val = virNWFilterVarValueCopy(ipaddr);
         if (!val)
-            return 1;
+            return -1;
 
         if (virHashAddEntry(table->hashTable,
                             NWFILTER_STD_VAR_IP,
                             val) < 0) {
             virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
                                    "%s", _("Could not add variable 'IP' to hashmap"));
-            return 1;
+            return -1;
         }
     }
 
@@ -200,7 +200,7 @@ virNWFilterCreateVarHashmap(char *macaddr,
         return NULL;
     }
 
-    if (virNWFilterVarHashmapAddStdValues(table, macaddr, ipaddr)) {
+    if (virNWFilterVarHashmapAddStdValues(table, macaddr, ipaddr) < 0) {
         virNWFilterHashTableFree(table);
         return NULL;
     }
@@ -339,10 +339,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;
@@ -404,13 +404,13 @@ _virNWFilterInstantiateRec(virNWFilterTechDriverPtr techdriver,
                                               ifname,
                                               vars);
             if (!inst) {
-                rc = 1;
+                rc = -1;
                 break;
             }
 
             if (VIR_REALLOC_N(*insts, (*nEntries)+1) < 0) {
                 virReportOOMError();
-                rc = 1;
+                rc = -1;
                 break;
             }
 
@@ -425,7 +425,7 @@ _virNWFilterInstantiateRec(virNWFilterTechDriverPtr techdriver,
                     virNWFilterReportError(VIR_ERR_NO_NWFILTER,
                                            _("Filter '%s' is in use."),
                                            inc->filterref);
-                    rc = 1;
+                    rc = -1;
                     virNWFilterObjUnlock(obj);
                     break;
                 }
@@ -436,7 +436,7 @@ _virNWFilterInstantiateRec(virNWFilterTechDriverPtr techdriver,
                                                                 vars);
                 if (!tmpvars) {
                     virReportOOMError();
-                    rc = 1;
+                    rc = -1;
                     virNWFilterObjUnlock(obj);
                     break;
                 }
@@ -467,13 +467,13 @@ _virNWFilterInstantiateRec(virNWFilterTechDriverPtr techdriver,
                 virNWFilterHashTableFree(tmpvars);
 
                 virNWFilterObjUnlock(obj);
-                if (rc)
+                if (rc < 0)
                     break;
             } else {
                 virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("referenced filter '%s' is missing"),
                                        inc->filterref);
-                rc = 1;
+                rc = -1;
                 break;
             }
         }
@@ -504,7 +504,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
                 if (!virHashLookup(vars->hashTable, rule->vars[j])) {
                     val = virNWFilterVarValueCreateSimpleCopyValue("1");
                     if (!val) {
-                        rc = 1;
+                        rc = -1;
                         break;
                     }
                     virNWFilterHashTablePut(missing_vars, rule->vars[j],
@@ -522,7 +522,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
                     virNWFilterReportError(VIR_ERR_NO_NWFILTER,
                                            _("Filter '%s' is in use."),
                                            inc->filterref);
-                    rc = 1;
+                    rc = -1;
                     virNWFilterObjUnlock(obj);
                     break;
                 }
@@ -533,7 +533,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
                                                                 vars);
                 if (!tmpvars) {
                     virReportOOMError();
-                    rc = 1;
+                    rc = -1;
                     virNWFilterObjUnlock(obj);
                     break;
                 }
@@ -559,13 +559,13 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
                 virNWFilterHashTableFree(tmpvars);
 
                 virNWFilterObjUnlock(obj);
-                if (rc)
+                if (rc < 0)
                     break;
             } else {
                 virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("referenced filter '%s' is missing"),
                                        inc->filterref);
-                rc = 1;
+                rc = -1;
                 break;
             }
         }
@@ -592,7 +592,7 @@ virNWFilterRuleInstancesToArray(int nEntries,
 
     if (VIR_ALLOC_N((*ptrs), (*nptrs)) < 0) {
         virReportOOMError();
-        return 1;
+        return -1;
     }
 
     (*nptrs) = 0;
@@ -649,7 +649,7 @@ virNWFilterInstantiate(virNWFilterTechDriverPtr techdriver,
     virNWFilterHashTablePtr missing_vars = virNWFilterHashTableCreate(0);
     if (!missing_vars) {
         virReportOOMError();
-        rc = 1;
+        rc = -1;
         goto err_exit;
     }
 
@@ -658,7 +658,7 @@ virNWFilterInstantiate(virNWFilterTechDriverPtr techdriver,
                                             missing_vars,
                                             useNewFilter,
                                             driver);
-    if (rc)
+    if (rc < 0)
         goto err_exit;
 
     if (virHashSize(missing_vars->hashTable) == 1) {
@@ -693,7 +693,7 @@ virNWFilterInstantiate(virNWFilterTechDriverPtr techdriver,
                                     useNewFilter, foundNewFilter,
                                     driver);
 
-    if (rc)
+    if (rc < 0)
         goto err_exit;
 
     switch (useNewFilter) {
@@ -709,10 +709,10 @@ virNWFilterInstantiate(virNWFilterTechDriverPtr techdriver,
 
         rc = virNWFilterRuleInstancesToArray(nEntries, insts,
                                              &ptrs, &nptrs);
-        if (rc)
+        if (rc < 0)
             goto err_exit;
 
-        if (virNWFilterLockIface(ifname))
+        if (virNWFilterLockIface(ifname) < 0)
             goto err_exit;
 
         rc = techdriver->applyNewRules(ifname, nptrs, ptrs);
@@ -724,7 +724,7 @@ virNWFilterInstantiate(virNWFilterTechDriverPtr techdriver,
             virResetLastError();
             /* interface changed/disppeared */
             techdriver->allTeardown(ifname);
-            rc = 1;
+            rc = -1;
         }
 
         virNWFilterUnlockIface(ifname);
@@ -752,7 +752,7 @@ err_unresolvable_vars:
         VIR_FREE(buf);
     }
 
-    rc = 1;
+    rc = -1;
     goto err_exit;
 }
 
@@ -792,7 +792,7 @@ __virNWFilterInstantiateFilter(bool teardownOld,
                                _("Could not get access to ACL tech "
                                "driver '%s'"),
                                drvname);
-        return 1;
+        return -1;
     }
 
     VIR_DEBUG("filter name: %s", filtername);
@@ -802,14 +802,14 @@ __virNWFilterInstantiateFilter(bool teardownOld,
         virNWFilterReportError(VIR_ERR_NO_NWFILTER,
                                _("Could not find filter '%s'"),
                                filtername);
-        return 1;
+        return -1;
     }
 
     if (obj->wantRemoved) {
         virNWFilterReportError(VIR_ERR_NO_NWFILTER,
                                _("Filter '%s' is in use."),
                                filtername);
-        rc = 1;
+        rc = -1;
         goto err_exit;
     }
 
@@ -817,7 +817,7 @@ __virNWFilterInstantiateFilter(bool teardownOld,
     str_macaddr = strdup(vmmacaddr);
     if (!str_macaddr) {
         virReportOOMError();
-        rc = 1;
+        rc = -1;
         goto err_exit;
     }
 
@@ -825,7 +825,7 @@ __virNWFilterInstantiateFilter(bool teardownOld,
 
     vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr);
     if (!vars1) {
-        rc = 1;
+        rc = -1;
         goto err_exit;
     }
 
@@ -835,7 +835,7 @@ __virNWFilterInstantiateFilter(bool teardownOld,
     vars = virNWFilterCreateVarsFrom(vars1,
                                      filterparams);
     if (!vars) {
-        rc = 1;
+        rc = -1;
         goto err_exit_vars1;
     }
 
@@ -955,7 +955,7 @@ virNWFilterInstantiateFilterLate(const char *ifname,
                                         driver,
                                         true,
                                         &foundNewFilter);
-    if (rc) {
+    if (rc < 0) {
         /* something went wrong... 'DOWN' the interface */
         if ((virNetDevValidateConfig(ifname, NULL, ifindex) <= 0) ||
             (virNetDevSetOnline(ifname, false) < 0)) {
@@ -1000,7 +1000,8 @@ virNWFilterUpdateInstantiateFilter(virConnectPtr conn,
     return rc;
 }
 
-int virNWFilterRollbackUpdateFilter(const virDomainNetDefPtr net)
+static int
+virNWFilterRollbackUpdateFilter(const virDomainNetDefPtr net)
 {
     const char *drvname = EBIPTABLES_DRIVER_ID;
     int ifindex;
@@ -1012,7 +1013,7 @@ int virNWFilterRollbackUpdateFilter(const virDomainNetDefPtr net)
                                _("Could not get access to ACL tech "
                                "driver '%s'"),
                                drvname);
-        return 1;
+        return -1;
     }
 
     /* don't tear anything while the address is being learned */
@@ -1025,7 +1026,7 @@ int virNWFilterRollbackUpdateFilter(const virDomainNetDefPtr net)
 }
 
 
-int
+static int
 virNWFilterTearOldFilter(virDomainNetDefPtr net)
 {
     const char *drvname = EBIPTABLES_DRIVER_ID;
@@ -1038,7 +1039,7 @@ virNWFilterTearOldFilter(virDomainNetDefPtr net)
                                _("Could not get access to ACL tech "
                                "driver '%s'"),
                                drvname);
-        return 1;
+        return -1;
     }
 
     /* don't tear anything while the address is being learned */
@@ -1063,13 +1064,13 @@ _virNWFilterTeardownFilter(const char *ifname)
                                _("Could not get access to ACL tech "
                                "driver '%s'"),
                                drvname);
-        return 1;
+        return -1;
     }
 
     virNWFilterTerminateLearnReq(ifname);
 
-    if (virNWFilterLockIface(ifname))
-       return 1;
+    if (virNWFilterLockIface(ifname) < 0)
+       return -1;
 
     techdriver->allTeardown(ifname);
 

src/nwfilter/nwfilter_gentech_driver.h

@@ -42,9 +42,6 @@ int virNWFilterInstantiateFilter(virConnectPtr conn,
 int virNWFilterUpdateInstantiateFilter(virConnectPtr conn,
                                        const virDomainNetDefPtr net,
                                        bool *skipIface);
-int virNWFilterRollbackUpdateFilter(const virDomainNetDefPtr net);
-
-int virNWFilterTearOldFilter(const virDomainNetDefPtr net);
 
 int virNWFilterInstantiateFilterLate(const char *ifname,
                                      int ifindex,

src/nwfilter/nwfilter_learnipaddr.c

@@ -149,7 +149,7 @@ virNWFilterLockIface(const char *ifname) {
             goto err_exit;
         }
 
-        if (virMutexInitRecursive(&ifaceLock->lock)) {
+        if (virMutexInitRecursive(&ifaceLock->lock) < 0) {
             virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                    _("mutex initialization failed"));
             VIR_FREE(ifaceLock);
@@ -184,7 +184,7 @@ virNWFilterLockIface(const char *ifname) {
  err_exit:
     virMutexUnlock(&ifaceMapLock);
 
-    return 1;
+    return -1;
 }
 
 
@@ -248,7 +248,7 @@ virNWFilterRegisterLearnReq(virNWFilterIPAddrLearnReqPtr req) {
 
 int
 virNWFilterTerminateLearnReq(const char *ifname) {
-    int rc = 1;
+    int rc = -1;
     int ifindex;
     virNWFilterIPAddrLearnReqPtr req;
 
@@ -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)
@@ -494,7 +491,7 @@ learnIPAddressThread(void *arg)
     enum howDetect howDetected = 0;
     virNWFilterTechDriverPtr techdriver = req->techdriver;
 
-    if (virNWFilterLockIface(req->ifname))
+    if (virNWFilterLockIface(req->ifname) < 0)
        goto err_no_lock;
 
     req->status = 0;
@@ -520,7 +517,7 @@ learnIPAddressThread(void *arg)
     case DETECT_DHCP:
         if (techdriver->applyDHCPOnlyRules(req->ifname,
                                            req->macaddr,
-                                           NULL, false)) {
+                                           NULL, false) < 0) {
             req->status = EINVAL;
             goto done;
         }
@@ -530,7 +527,7 @@ learnIPAddressThread(void *arg)
         break;
     default:
         if (techdriver->applyBasicRules(req->ifname,
-                                        req->macaddr)) {
+                                        req->macaddr) < 0) {
             req->status = EINVAL;
             goto done;
         }
@@ -701,7 +698,7 @@ learnIPAddressThread(void *arg)
         sa.data.inet4.sin_addr.s_addr = vmaddr;
         char *inetaddr;
 
-        if ((inetaddr = virSocketAddrFormat(&sa))!= NULL) {
+        if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) {
             if (virNWFilterAddIpAddrForIfname(req->ifname, inetaddr) < 0) {
                 VIR_ERROR(_("Failed to add IP address %s to IP address "
                           "cache for interface %s"), inetaddr, req->ifname);
@@ -781,14 +778,14 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver,
     virNWFilterHashTablePtr ht = NULL;
 
     if (howDetect == 0)
-        return 1;
+        return -1;
 
     if ( !techdriver->canApplyBasicRules()) {
         virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("IP parameter must be provided since "
                                  "snooping the IP address does not work "
                                  "possibly due to missing tools"));
-        return 1;
+        return -1;
     }
 
     if (VIR_ALLOC(req) < 0) {
@@ -802,7 +799,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);
@@ -838,7 +835,7 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver,
 
     rc = virNWFilterRegisterLearnReq(req);
 
-    if (rc)
+    if (rc < 0)
         goto err_free_req;
 
     if (pthread_create(&req->thread,
@@ -856,7 +853,7 @@ err_free_ht:
 err_free_req:
     virNWFilterIPAddrLearnReqFree(req);
 err_no_req:
-    return 1;
+    return -1;
 }
 
 #else
@@ -876,7 +873,7 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver ATTRIBUTE_UNUSED,
                            _("IP parameter must be given since libvirt "
                              "was not compiled with IP address learning "
                              "support"));
-    return 1;
+    return -1;
 }
 #endif /* HAVE_LIBPCAP */
 
@@ -895,35 +892,35 @@ virNWFilterLearnInit(void) {
 
     pendingLearnReq = virHashCreate(0, freeLearnReqEntry);
     if (!pendingLearnReq) {
-        return 1;
+        return -1;
     }
 
-    if (virMutexInit(&pendingLearnReqLock)) {
+    if (virMutexInit(&pendingLearnReqLock) < 0) {
         virNWFilterLearnShutdown();
-        return 1;
+        return -1;
     }
 
     ipAddressMap = virNWFilterHashTableCreate(0);
     if (!ipAddressMap) {
         virReportOOMError();
         virNWFilterLearnShutdown();
-        return 1;
+        return -1;
     }
 
-    if (virMutexInit(&ipAddressMapLock)) {
+    if (virMutexInit(&ipAddressMapLock) < 0) {
         virNWFilterLearnShutdown();
-        return 1;
+        return -1;
     }
 
     ifaceLockMap = virHashCreate(0, freeIfaceLock);
     if (!ifaceLockMap) {
         virNWFilterLearnShutdown();
-        return 1;
+        return -1;
     }
 
-    if (virMutexInit(&ifaceMapLock)) {
+    if (virMutexInit(&ifaceMapLock) < 0) {
         virNWFilterLearnShutdown();
-        return 1;
+        return -1;
     }
 
     return 0;

src/qemu/qemu_command.c

@@ -275,8 +275,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
 
     if (tapfd >= 0) {
         if ((net->filter) && (net->ifname)) {
-            err = virDomainConfNWFilterInstantiate(conn, net);
-            if (err)
+            if (virDomainConfNWFilterInstantiate(conn, net) < 0)
                 VIR_FORCE_CLOSE(tapfd);
         }
     }

src/qemu/qemu_process.c

@@ -2355,7 +2355,7 @@ qemuProcessFiltersInstantiate(virConnectPtr conn,
     for (i = 0 ; i < def->nnets ; i++) {
         virDomainNetDefPtr net = def->nets[i];
         if ((net->filter) && (net->ifname)) {
-           if (virDomainConfNWFilterInstantiate(conn, net)) {
+           if (virDomainConfNWFilterInstantiate(conn, net) < 0) {
                 err = 1;
                 break;
             }

src/uml/uml_conf.c

@@ -148,7 +148,7 @@ umlConnectTapDevice(virConnectPtr conn,
     }
 
     if (net->filter) {
-        if (virDomainConfNWFilterInstantiate(conn, net)) {
+        if (virDomainConfNWFilterInstantiate(conn, net) < 0) {
             if (template_ifname)
                 VIR_FREE(net->ifname);
             goto error;