[libvirt PATCH v2 12/15] nwfilter: use standard label names when reasonable

Laine Stump posted 15 patches 4 years, 4 months ago
[libvirt PATCH v2 12/15] nwfilter: use standard label names when reasonable
Posted by Laine Stump 4 years, 4 months ago
Rather than having labels named exit, done, exit_snooprequnlock,
skip_rename, etc, use the standard "cleanup" label. And instead of
err_exit, malformed, tear_down_tmpebchains, use "error".

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/nwfilter/nwfilter_dhcpsnoop.c         | 36 +++++++++++------------
 src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++++----
 src/nwfilter/nwfilter_gentech_driver.c    | 32 ++++++++++----------
 src/nwfilter/nwfilter_learnipaddr.c       | 24 +++++++--------
 4 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index f530341872..6de41ff209 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -456,11 +456,11 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
     virNWFilterSnoopReqLock(req);
 
     if (virNWFilterIPAddrMapAddIPAddr(req->binding->portdevname, ipaddr) < 0)
-        goto exit_snooprequnlock;
+        goto cleanup;
 
     if (!instantiate) {
         rc = 0;
-        goto exit_snooprequnlock;
+        goto cleanup;
     }
 
     /* instantiate the filters */
@@ -471,7 +471,7 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
                                               req->ifindex);
     }
 
- exit_snooprequnlock:
+ cleanup:
     virNWFilterSnoopReqUnlock(req);
 
     VIR_FREE(ipaddr);
@@ -732,7 +732,7 @@ virNWFilterSnoopReqLeaseAdd(virNWFilterSnoopReqPtr req,
 
         virNWFilterSnoopReqUnlock(req);
 
-        goto exit;
+        goto cleanup;
     }
 
     virNWFilterSnoopReqUnlock(req);
@@ -757,7 +757,7 @@ virNWFilterSnoopReqLeaseAdd(virNWFilterSnoopReqPtr req,
 
     g_atomic_int_add(&virNWFilterSnoopState.nLeases, 1);
 
- exit:
+ cleanup:
     if (update_leasefile)
         virNWFilterSnoopLeaseFileSave(pl);
 
@@ -902,7 +902,7 @@ virNWFilterSnoopDHCPGetOpt(virNWFilterSnoopDHCPHdrPtr pd, int len,
         switch (pd->d_opts[oind]) {
         case DHCPO_LEASE:
             if (olen - oind < 6)
-                goto malformed;
+                goto error;
             if (*pleasetime)
                 return -1;  /* duplicate lease time */
             memcpy(&nwint, (char *)pd->d_opts + oind + 2, sizeof(nwint));
@@ -910,7 +910,7 @@ virNWFilterSnoopDHCPGetOpt(virNWFilterSnoopDHCPHdrPtr pd, int len,
             break;
         case DHCPO_MTYPE:
             if (olen - oind < 3)
-                goto malformed;
+                goto error;
             if (*pmtype)
                 return -1;  /* duplicate message type */
             *pmtype = pd->d_opts[oind + 2];
@@ -922,12 +922,12 @@ virNWFilterSnoopDHCPGetOpt(virNWFilterSnoopDHCPHdrPtr pd, int len,
             return 0;
         default:
             if (olen - oind < 2)
-                goto malformed;
+                goto error;
         }
         oind += pd->d_opts[oind + 1] + 2;
     }
     return 0;
- malformed:
+ error:
     VIR_WARN("got lost in the options!");
     return -1;
 }
@@ -1386,7 +1386,7 @@ virNWFilterDHCPSnoopThread(void *req0)
     virNWFilterSnoopReqUnlock(req);
 
     if (req->threadStatus != THREAD_STATUS_OK)
-        goto exit;
+        goto cleanup;
 
     while (!error) {
         if (virNWFilterSnoopAdjustPoll(pcapConf,
@@ -1414,7 +1414,7 @@ virNWFilterDHCPSnoopThread(void *req0)
          */
         if (!virNWFilterSnoopIsActive(threadkey) ||
             req->jobCompletionStatus != 0)
-            goto exit;
+            goto cleanup;
 
         for (i = 0; n > 0 && i < G_N_ELEMENTS(fds); i++) {
             if (!fds[i].revents)
@@ -1531,7 +1531,7 @@ virNWFilterDHCPSnoopThread(void *req0)
     virNWFilterSnoopReqUnlock(req);
     virNWFilterSnoopUnlock();
 
- exit:
+ cleanup:
     virThreadPoolFree(worker);
 
     virNWFilterSnoopReqPut(req);
@@ -1774,14 +1774,14 @@ virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLeasePtr ipl)
         virNWFilterSnoopLeaseFileOpen();
     if (virNWFilterSnoopLeaseFileWrite(virNWFilterSnoopState.leaseFD,
                                        req->ifkey, ipl) < 0)
-        goto err_exit;
+        goto error;
 
     /* keep dead leases at < ~95% of file size */
     if (g_atomic_int_add(&virNWFilterSnoopState.wLeases, 1) >=
         g_atomic_int_get(&virNWFilterSnoopState.nLeases) * 20)
         virNWFilterSnoopLeaseFileLoad();   /* load & refresh lease file */
 
- err_exit:
+ error:
     virNWFilterSnoopUnlock();
 }
 
@@ -1876,7 +1876,7 @@ virNWFilterSnoopLeaseFileRefresh(void)
     if (VIR_CLOSE(tfd) < 0) {
         virReportSystemError(errno, _("unable to close %s"), TMPLEASEFILE);
         /* assuming the old lease file is still better, skip the renaming */
-        goto skip_rename;
+        goto cleanup;
     }
 
     if (rename(TMPLEASEFILE, LEASEFILE) < 0) {
@@ -1886,7 +1886,7 @@ virNWFilterSnoopLeaseFileRefresh(void)
     }
     g_atomic_int_set(&virNWFilterSnoopState.wLeases, 0);
 
- skip_rename:
+ cleanup:
     virNWFilterSnoopLeaseFileOpen();
 }
 
@@ -2051,14 +2051,14 @@ virNWFilterDHCPSnoopInit(void)
     if (!virNWFilterSnoopState.ifnameToKey ||
         !virNWFilterSnoopState.snoopReqs ||
         !virNWFilterSnoopState.active)
-        goto err_exit;
+        goto error;
 
     virNWFilterSnoopLeaseFileLoad();
     virNWFilterSnoopLeaseFileOpen();
 
     return 0;
 
- err_exit:
+ error:
     virHashFree(virNWFilterSnoopState.ifnameToKey);
     virNWFilterSnoopState.ifnameToKey = NULL;
 
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 94eaac927a..8ac3a7271e 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -2893,11 +2893,11 @@ ebtablesApplyBasicRules(const char *ifname,
     ebtablesRenameTmpRootChainFW(fw, true, ifname);
 
     if (virFirewallApply(fw) < 0)
-        goto tear_down_tmpebchains;
+        goto error;
 
     return 0;
 
- tear_down_tmpebchains:
+ error:
     ebtablesCleanAll(ifname);
     return -1;
 }
@@ -3009,11 +3009,11 @@ ebtablesApplyDHCPOnlyRules(const char *ifname,
     }
 
     if (virFirewallApply(fw) < 0)
-        goto tear_down_tmpebchains;
+        goto error;
 
     return 0;
 
- tear_down_tmpebchains:
+ error:
     ebtablesCleanAll(ifname);
     return -1;
 }
@@ -3060,11 +3060,11 @@ ebtablesApplyDropAllRules(const char *ifname)
     ebtablesRenameTmpRootChainFW(fw, false, ifname);
 
     if (virFirewallApply(fw) < 0)
-        goto tear_down_tmpebchains;
+        goto error;
 
     return 0;
 
- tear_down_tmpebchains:
+ error:
     ebtablesCleanAll(ifname);
     return -1;
 }
diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index aff42cbfb0..400d064724 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -204,14 +204,14 @@ virNWFilterCreateVarsFrom(virHashTablePtr vars1,
         return NULL;
 
     if (virNWFilterHashTablePutAll(vars1, res) < 0)
-        goto err_exit;
+        goto error;
 
     if (virNWFilterHashTablePutAll(vars2, res) < 0)
-        goto err_exit;
+        goto error;
 
     return res;
 
- err_exit:
+ error:
     virHashFree(res);
     return NULL;
 }
@@ -527,7 +527,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
 
     if (!missing_vars) {
         rc = -1;
-        goto err_exit;
+        goto error;
     }
 
     rc = virNWFilterDetermineMissingVarsRec(filter,
@@ -536,7 +536,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
                                             useNewFilter,
                                             driver);
     if (rc < 0)
-        goto err_exit;
+        goto error;
 
     lv = virHashLookup(binding->filterparams, NWFILTER_VARNAME_CTRL_IP_LEARNING);
     if (lv)
@@ -558,7 +558,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
                 rc = virNWFilterDHCPSnoopReq(techdriver,
                                              binding,
                                              driver);
-                goto err_exit;
+                goto error;
             } else if (STRCASEEQ(learning, "any")) {
                 if (!virNWFilterHasLearnReq(ifindex)) {
                     rc = virNWFilterLearnIPAddress(techdriver,
@@ -567,14 +567,14 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
                                                    driver,
                                                    DETECT_DHCP|DETECT_STATIC);
                 }
-                goto err_exit;
+                goto error;
             } else {
                 rc = -1;
                 virReportError(VIR_ERR_PARSE_FAILED,
                                _("filter '%s' "
                                  "learning value '%s' invalid."),
                                filter->name, learning);
-                goto err_exit;
+                goto error;
             }
         } else {
             goto err_unresolvable_vars;
@@ -583,7 +583,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
         goto err_unresolvable_vars;
     } else if (!forceWithPendingReq &&
                virNWFilterHasLearnReq(ifindex)) {
-        goto err_exit;
+        goto error;
     }
 
     rc = virNWFilterDefToInst(driver,
@@ -593,7 +593,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
                               &inst);
 
     if (rc < 0)
-        goto err_exit;
+        goto error;
 
     switch (useNewFilter) {
     case INSTANTIATE_FOLLOW_NEWFILTER:
@@ -606,7 +606,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
 
     if (instantiate) {
         if (virNWFilterLockIface(binding->portdevname) < 0)
-            goto err_exit;
+            goto error;
 
         rc = techdriver->applyNewRules(binding->portdevname, inst.rules, inst.nrules);
 
@@ -623,7 +623,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
         virNWFilterUnlockIface(binding->portdevname);
     }
 
- err_exit:
+ error:
     virNWFilterInstReset(&inst);
     virHashFree(missing_vars);
 
@@ -640,7 +640,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
     }
 
     rc = -1;
-    goto err_exit;
+    goto error;
 }
 
 
@@ -707,14 +707,14 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
     if (virNWFilterVarHashmapAddStdValue(binding->filterparams,
                                          NWFILTER_STD_VAR_MAC,
                                          vmmacaddr) < 0)
-        goto err_exit;
+        goto error;
 
     ipaddr = virNWFilterIPAddrMapGetIPAddr(binding->portdevname);
     if (ipaddr &&
         virNWFilterVarHashmapAddStdValue(binding->filterparams,
                                          NWFILTER_STD_VAR_IP,
                                          virNWFilterVarValueGetSimple(ipaddr)) < 0)
-        goto err_exit;
+        goto error;
 
 
     filter = virNWFilterObjGetDef(obj);
@@ -737,7 +737,7 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
                                   teardownOld, driver,
                                   forceWithPendingReq);
 
- err_exit:
+ error:
     virNWFilterObjUnlock(obj);
 
     return rc;
diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index 14c66cff35..95e21050b4 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -152,13 +152,13 @@ virNWFilterLockIface(const char *ifname)
     ifaceLock = virHashLookup(ifaceLockMap, ifname);
     if (!ifaceLock) {
         if (VIR_ALLOC(ifaceLock) < 0)
-            goto err_exit;
+            goto error;
 
         if (virMutexInitRecursive(&ifaceLock->lock) < 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("mutex initialization failed"));
             VIR_FREE(ifaceLock);
-            goto err_exit;
+            goto error;
         }
 
         if (virStrcpyStatic(ifaceLock->ifname, ifname) < 0) {
@@ -167,12 +167,12 @@ virNWFilterLockIface(const char *ifname)
                              "buffer "),
                            ifaceLock->ifname);
             VIR_FREE(ifaceLock);
-            goto err_exit;
+            goto error;
         }
 
         while (virHashAddEntry(ifaceLockMap, ifname, ifaceLock)) {
             VIR_FREE(ifaceLock);
-            goto err_exit;
+            goto error;
         }
 
         ifaceLock->refctr = 0;
@@ -186,7 +186,7 @@ virNWFilterLockIface(const char *ifname)
 
     return 0;
 
- err_exit:
+ error:
     virMutexUnlock(&ifaceMapLock);
 
     return -1;
@@ -414,7 +414,7 @@ learnIPAddressThread(void *arg)
     if (virNetDevValidateConfig(req->binding->portdevname, NULL, req->ifindex) <= 0) {
         virResetLastError();
         req->status = ENODEV;
-        goto done;
+        goto cleanup;
     }
 
     handle = pcap_open_live(listen_if, BUFSIZ, 0, PKT_TIMEOUT_MS, errbuf);
@@ -422,7 +422,7 @@ learnIPAddressThread(void *arg)
     if (handle == NULL) {
         VIR_DEBUG("Couldn't open device %s: %s", listen_if, errbuf);
         req->status = ENODEV;
-        goto done;
+        goto cleanup;
     }
 
     fds[0].fd = pcap_fileno(handle);
@@ -436,7 +436,7 @@ learnIPAddressThread(void *arg)
                                            NULL, false) < 0) {
             VIR_DEBUG("Unable to apply DHCP only rules");
             req->status = EINVAL;
-            goto done;
+            goto cleanup;
         }
         virBufferAddLit(&buf, "src port 67 and dst port 68");
     } else {
@@ -444,7 +444,7 @@ learnIPAddressThread(void *arg)
                                         &req->binding->mac) < 0) {
             VIR_DEBUG("Unable to apply basic rules");
             req->status = EINVAL;
-            goto done;
+            goto cleanup;
         }
         virBufferAsprintf(&buf, "ether host %s or ether dst ff:ff:ff:ff:ff:ff",
                           macaddr);
@@ -455,14 +455,14 @@ learnIPAddressThread(void *arg)
     if (pcap_compile(handle, &fp, filter, 1, 0) != 0) {
         VIR_DEBUG("Couldn't compile filter '%s'", filter);
         req->status = EINVAL;
-        goto done;
+        goto cleanup;
     }
 
     if (pcap_setfilter(handle, &fp) != 0) {
         VIR_DEBUG("Couldn't set filter '%s'", filter);
         req->status = EINVAL;
         pcap_freecode(&fp);
-        goto done;
+        goto cleanup;
     }
 
     pcap_freecode(&fp);
@@ -622,7 +622,7 @@ learnIPAddressThread(void *arg)
         }
     } /* while */
 
- done:
+ cleanup:
     VIR_FREE(filter);
 
     if (handle)
-- 
2.25.4

Re: [libvirt PATCH v2 12/15] nwfilter: use standard label names when reasonable
Posted by Ján Tomko 4 years, 4 months ago
On a Tuesday in 2020, Laine Stump wrote:
>Rather than having labels named exit, done, exit_snooprequnlock,
>skip_rename, etc, use the standard "cleanup" label. And instead of
>err_exit, malformed, tear_down_tmpebchains, use "error".
>
>Signed-off-by: Laine Stump <laine@redhat.com>
>---
> src/nwfilter/nwfilter_dhcpsnoop.c         | 36 +++++++++++------------
> src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++++----
> src/nwfilter/nwfilter_gentech_driver.c    | 32 ++++++++++----------
> src/nwfilter/nwfilter_learnipaddr.c       | 24 +++++++--------
> 4 files changed, 52 insertions(+), 52 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano