From fae0a0e5a43d23ecc42d42625c9057af83bf8461 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Wed, 24 Jun 2020 21:59:39 -0400 Subject: [PATCH] nwfilter: convert local pointers to use g_auto* MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Laine Stump Reviewed-by: Ján Tomko --- src/nwfilter/nwfilter_dhcpsnoop.c | 91 +++++++---------------- src/nwfilter/nwfilter_ebiptables_driver.c | 75 +++++++------------ src/nwfilter/nwfilter_gentech_driver.c | 15 ++-- src/nwfilter/nwfilter_learnipaddr.c | 7 +- 4 files changed, 61 insertions(+), 127 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 4bc1607694..64671af135 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -292,18 +292,17 @@ static const unsigned char dhcp_magic[4] = { 99, 130, 83, 99 }; static char * virNWFilterSnoopActivate(virNWFilterSnoopReqPtr req) { - char *key; - - key = g_strdup_printf("%p-%d", req, req->ifindex); + g_autofree char *key = g_strdup_printf("%p-%d", req, req->ifindex); + char *ret = NULL; virNWFilterSnoopActiveLock(); - if (virHashAddEntry(virNWFilterSnoopState.active, key, (void *)0x1) < 0) - VIR_FREE(key); + if (virHashAddEntry(virNWFilterSnoopState.active, key, (void *)0x1) == 0) + ret = g_steal_pointer(&key); virNWFilterSnoopActiveUnlock(); - return key; + return ret; } static void @@ -442,11 +441,10 @@ static int virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl, bool instantiate) { - char *ipaddr; + g_autofree char *ipaddr = virSocketAddrFormat(&ipl->ipAddress); int rc = -1; virNWFilterSnoopReqPtr req; - ipaddr = virSocketAddrFormat(&ipl->ipAddress); if (!ipaddr) return -1; @@ -473,9 +471,6 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl, cleanup: virNWFilterSnoopReqUnlock(req); - - VIR_FREE(ipaddr); - return rc; } @@ -551,7 +546,7 @@ virNWFilterSnoopReqGet(virNWFilterSnoopReqPtr req) static virNWFilterSnoopReqPtr virNWFilterSnoopReqNew(const char *ifkey) { - virNWFilterSnoopReqPtr req; + g_autofree virNWFilterSnoopReqPtr req = g_new0(virNWFilterSnoopReq, 1); if (ifkey == NULL || strlen(ifkey) != VIR_IFKEY_LEN - 1) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -562,28 +557,20 @@ virNWFilterSnoopReqNew(const char *ifkey) return NULL; } - req = g_new0(virNWFilterSnoopReq, 1); - req->threadStatus = THREAD_STATUS_NONE; - if (virStrcpyStatic(req->ifkey, ifkey) < 0|| - virMutexInitRecursive(&req->lock) < 0) - goto err_free_req; + if (virStrcpyStatic(req->ifkey, ifkey) < 0 || + virMutexInitRecursive(&req->lock) < 0) { + return NULL; + } - if (virCondInit(&req->threadStatusCond) < 0) - goto err_destroy_mutex; + if (virCondInit(&req->threadStatusCond) < 0) { + virMutexDestroy(&req->lock); + return NULL; + } virNWFilterSnoopReqGet(req); - - return req; - - err_destroy_mutex: - virMutexDestroy(&req->lock); - - err_free_req: - VIR_FREE(req); - - return NULL; + return g_steal_pointer(&req); } /* @@ -815,7 +802,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, { int ret = 0; virNWFilterSnoopIPLeasePtr ipl; - char *ipstr = NULL; + g_autofree char *ipstr = NULL; /* protect req->start, req->ifname and the lease */ virNWFilterSnoopReqLock(req); @@ -868,8 +855,6 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, ignore_value(!!g_atomic_int_dec_and_test(&virNWFilterSnoopState.nLeases)); lease_not_found: - VIR_FREE(ipstr); - virNWFilterSnoopReqUnlock(req); return ret; @@ -1045,7 +1030,7 @@ virNWFilterSnoopDHCPOpen(const char *ifname, virMacAddr *mac, pcap_t *handle = NULL; struct bpf_program fp; char pcap_errbuf[PCAP_ERRBUF_SIZE]; - char *ext_filter = NULL; + g_autofree char *ext_filter = NULL; char macaddr[VIR_MAC_STRING_BUFLEN]; virMacAddrFormat(mac, macaddr); @@ -1075,7 +1060,7 @@ virNWFilterSnoopDHCPOpen(const char *ifname, virMacAddr *mac, if (handle == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("pcap_create failed")); - goto cleanup_nohandle; + return NULL; } if (pcap_set_snaplen(handle, PCAP_PBUFSIZE) < 0 || @@ -1107,17 +1092,12 @@ virNWFilterSnoopDHCPOpen(const char *ifname, virMacAddr *mac, } pcap_freecode(&fp); - VIR_FREE(ext_filter); - return handle; cleanup_freecode: pcap_freecode(&fp); cleanup: pcap_close(handle); - cleanup_nohandle: - VIR_FREE(ext_filter); - return NULL; } @@ -1128,7 +1108,7 @@ virNWFilterSnoopDHCPOpen(const char *ifname, virMacAddr *mac, static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque) { virNWFilterSnoopReqPtr req = opaque; - virNWFilterDHCPDecodeJobPtr job = jobdata; + g_autofree virNWFilterDHCPDecodeJobPtr job = jobdata; virNWFilterSnoopEthHdrPtr packet = (virNWFilterSnoopEthHdrPtr)job->packet; if (virNWFilterSnoopDHCPDecode(req, packet, @@ -1140,7 +1120,6 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque) "interface '%s'"), req->binding->portdevname); } ignore_value(!!g_atomic_int_dec_and_test(job->qCtr)); - VIR_FREE(job); } /* @@ -1307,7 +1286,7 @@ virNWFilterDHCPSnoopThread(void *req0) int errcount = 0; int tmp = -1, rv, n, pollTo; size_t i; - char *threadkey = NULL; + g_autofree char *threadkey = NULL; virThreadPoolPtr worker = NULL; time_t last_displayed = 0, last_displayed_queue = 0; virNWFilterSnoopPcapConf pcapConf[] = { @@ -1533,8 +1512,6 @@ virNWFilterDHCPSnoopThread(void *req0) virNWFilterSnoopReqPut(req); - VIR_FREE(threadkey); - for (i = 0; i < G_N_ELEMENTS(pcapConf); i++) { if (pcapConf[i].handle) pcap_close(pcapConf[i].handle); @@ -1721,18 +1698,13 @@ static int virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey, virNWFilterSnoopIPLeasePtr ipl) { - char *lbuf = NULL; - char *ipstr, *dhcpstr; + g_autofree char *lbuf = NULL; + g_autofree char *ipstr = virSocketAddrFormat(&ipl->ipAddress); + g_autofree char *dhcpstr = virSocketAddrFormat(&ipl->ipServer); int len; - int ret = 0; - ipstr = virSocketAddrFormat(&ipl->ipAddress); - dhcpstr = virSocketAddrFormat(&ipl->ipServer); - - if (!dhcpstr || !ipstr) { - ret = -1; - goto cleanup; - } + if (!dhcpstr || !ipstr) + return -1; /* time intf ip dhcpserver */ lbuf = g_strdup_printf("%u %s %s %s\n", ipl->timeout, ifkey, ipstr, dhcpstr); @@ -1740,18 +1712,11 @@ virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey, if (safewrite(lfd, lbuf, len) != len) { virReportSystemError(errno, "%s", _("lease file write failed")); - ret = -1; - goto cleanup; + return -1; } ignore_value(g_fsync(lfd)); - - cleanup: - VIR_FREE(lbuf); - VIR_FREE(dhcpstr); - VIR_FREE(ipstr); - - return ret; + return 0; } /* diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 177e7e62b9..9c9d63f14b 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -188,10 +188,10 @@ _printDataType(virNWFilterVarCombIterPtr vars, bool asHex, bool directionIn) { bool done; - char *data; + g_autofree char *data = NULL; uint8_t ctr; g_auto(virBuffer) vb = VIR_BUFFER_INITIALIZER; - char *flags; + g_autofree char *flags = NULL; if (printVar(vars, buf, bufsize, item, &done) < 0) return -1; @@ -207,10 +207,8 @@ _printDataType(virNWFilterVarCombIterPtr vars, if (g_snprintf(buf, bufsize, "%s", data) >= bufsize) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("buffer too small for IP address")); - VIR_FREE(data); return -1; } - VIR_FREE(data); break; case DATATYPE_IPV6ADDR: @@ -221,10 +219,8 @@ _printDataType(virNWFilterVarCombIterPtr vars, if (g_snprintf(buf, bufsize, "%s", data) >= bufsize) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("buffer too small for IPv6 address")); - VIR_FREE(data); return -1; } - VIR_FREE(data); break; case DATATYPE_MACADDR: @@ -308,10 +304,8 @@ _printDataType(virNWFilterVarCombIterPtr vars, if (virStrcpy(buf, flags, bufsize) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Buffer too small for IPSETFLAGS type")); - VIR_FREE(flags); return -1; } - VIR_FREE(flags); break; case DATATYPE_STRING: @@ -1187,19 +1181,19 @@ _iptablesCreateRuleInstance(virFirewallPtr fw, return -1; if (HAS_ENTRY_ITEM(&rule->p.tcpHdrFilter.dataTCPFlags)) { - char *flags; + g_autofree char *mask = NULL; + g_autofree char *flags = NULL; if (ENTRY_WANT_NEG_SIGN(&rule->p.tcpHdrFilter.dataTCPFlags)) virFirewallRuleAddArg(fw, fwrule, "!"); virFirewallRuleAddArg(fw, fwrule, "--tcp-flags"); - if (!(flags = virNWFilterPrintTCPFlags(rule->p.tcpHdrFilter.dataTCPFlags.u.tcpFlags.mask))) + if (!(mask = virNWFilterPrintTCPFlags(rule->p.tcpHdrFilter.dataTCPFlags.u.tcpFlags.mask))) return -1; - virFirewallRuleAddArg(fw, fwrule, flags); - VIR_FREE(flags); + virFirewallRuleAddArg(fw, fwrule, mask); + if (!(flags = virNWFilterPrintTCPFlags(rule->p.tcpHdrFilter.dataTCPFlags.u.tcpFlags.flags))) return -1; virFirewallRuleAddArg(fw, fwrule, flags); - VIR_FREE(flags); } if (iptablesHandlePortData(fw, fwrule, @@ -1548,7 +1542,9 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, bool directionIn = false; char chainPrefix[2]; bool maySkipICMP, inout = false; - char *matchState = NULL; + g_autofree char *matchState1 = NULL; + g_autofree char *matchState2 = NULL; + g_autofree char *matchState3 = NULL; bool create; if ((rule->tt == VIR_NWFILTER_RULE_DIRECTION_IN) || @@ -1562,7 +1558,6 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, maySkipICMP = directionIn || inout; create = true; - matchState = NULL; if (directionIn && !inout) { if ((rule->flags & IPTABLES_STATE_FLAGS)) @@ -1570,7 +1565,7 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, } if (create && (rule->flags & IPTABLES_STATE_FLAGS)) { - if (printStateMatchFlags(rule->flags, &matchState) < 0) + if (printStateMatchFlags(rule->flags, &matchState1) < 0) return -1; } @@ -1583,11 +1578,10 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, rule, ifname, vars, - matchState, false, + matchState1, false, "RETURN", maySkipICMP); - VIR_FREE(matchState); if (rc < 0) return rc; } @@ -1601,7 +1595,7 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, } if (create && (rule->flags & IPTABLES_STATE_FLAGS)) { - if (printStateMatchFlags(rule->flags, &matchState) < 0) + if (printStateMatchFlags(rule->flags, &matchState2) < 0) return -1; } @@ -1614,12 +1608,9 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, rule, ifname, vars, - matchState, false, + matchState2, false, "ACCEPT", maySkipICMP); - - VIR_FREE(matchState); - if (rc < 0) return rc; } @@ -1633,7 +1624,7 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, create = false; } else { if ((rule->flags & IPTABLES_STATE_FLAGS)) { - if (printStateMatchFlags(rule->flags, &matchState) < 0) + if (printStateMatchFlags(rule->flags, &matchState3) < 0) return -1; } } @@ -1648,10 +1639,9 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, rule, ifname, vars, - matchState, false, + matchState3, false, "RETURN", maySkipICMP); - VIR_FREE(matchState); } return rc; @@ -1797,7 +1787,6 @@ ebtablesCreateRuleInstance(virFirewallPtr fw, const char *target; bool hasMask = false; virFirewallRulePtr fwrule; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; if (STREQ(chainSuffix, virNWFilterChainSuffixTypeToString( @@ -2320,7 +2309,8 @@ ebtablesCreateRuleInstance(virFirewallPtr fw, HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.dataICMPCodeStart) || HAS_ENTRY_ITEM(&rule->p.ipv6HdrFilter.dataICMPCodeEnd)) { bool lo = false; - char *r; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_autofree char *r = NULL; virFirewallRuleAddArg(fw, fwrule, "--ip6-icmp-type"); @@ -2385,8 +2375,6 @@ ebtablesCreateRuleInstance(virFirewallPtr fw, r = virBufferContentAndReset(&buf); virFirewallRuleAddArg(fw, fwrule, r); - - VIR_FREE(r); } break; @@ -3295,9 +3283,8 @@ ebtablesGetSubChainInsts(virHashTablePtr chains, ebtablesSubChainInstPtr **insts, size_t *ninsts) { - virHashKeyValuePairPtr filter_names; + g_autofree virHashKeyValuePairPtr filter_names = NULL; size_t i; - int ret = -1; filter_names = virHashGetItems(chains, ebiptablesFilterOrderSort); @@ -3305,7 +3292,7 @@ ebtablesGetSubChainInsts(virHashTablePtr chains, return -1; for (i = 0; filter_names[i].key; i++) { - ebtablesSubChainInstPtr inst; + g_autofree ebtablesSubChainInstPtr inst = NULL; enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername( filter_names[i].key); @@ -3318,18 +3305,11 @@ ebtablesGetSubChainInsts(virHashTablePtr chains, inst->protoidx = idx; inst->filtername = filter_names[i].key; - if (VIR_APPEND_ELEMENT(*insts, *ninsts, inst) < 0) { - VIR_FREE(inst); - goto cleanup; - } + if (VIR_APPEND_ELEMENT(*insts, *ninsts, inst) < 0) + return -1; } - ret = 0; - - cleanup: - VIR_FREE(filter_names); - return ret; - + return 0; } static int @@ -3339,12 +3319,12 @@ ebiptablesApplyNewRules(const char *ifname, { size_t i, j; g_autoptr(virFirewall) fw = virFirewallNew(); - virHashTablePtr chains_in_set = virHashCreate(10, NULL); - virHashTablePtr chains_out_set = virHashCreate(10, NULL); + g_autoptr(virHashTable) chains_in_set = virHashCreate(10, NULL); + g_autoptr(virHashTable) chains_out_set = virHashCreate(10, NULL); bool haveEbtables = false; bool haveIptables = false; bool haveIp6tables = false; - ebtablesSubChainInstPtr *subchains = NULL; + g_autofree ebtablesSubChainInstPtr *subchains = NULL; size_t nsubchains = 0; int ret = -1; @@ -3538,9 +3518,6 @@ ebiptablesApplyNewRules(const char *ifname, cleanup: for (i = 0; i < nsubchains; i++) VIR_FREE(subchains[i]); - VIR_FREE(subchains); - virHashFree(chains_in_set); - virHashFree(chains_out_set); return ret; } diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index acd5614987..071f15caea 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -415,7 +415,6 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, virNWFilterDefPtr next_filter; virNWFilterDefPtr newNext_filter; virNWFilterVarValuePtr val; - virHashTablePtr tmpvars; for (i = 0; i < filter->nentries; i++) { virNWFilterRuleDefPtr rule = filter->filterEntries[i]->rule; @@ -425,18 +424,16 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, for (j = 0; j < rule->nVarAccess; j++) { if (!virNWFilterVarAccessIsAvailable(rule->varAccess[j], vars)) { - char *varAccess; + g_autofree char *varAccess = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virNWFilterVarAccessPrint(rule->varAccess[j], &buf); - val = virNWFilterVarValueCreateSimpleCopyValue("1"); - if (!val) + if (!(val = virNWFilterVarValueCreateSimpleCopyValue("1"))) return -1; varAccess = virBufferContentAndReset(&buf); rc = virHashUpdateEntry(missing_vars, varAccess, val); - VIR_FREE(varAccess); if (rc < 0) { virNWFilterVarValueFree(val); return -1; @@ -444,6 +441,8 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, } } } else if (inc) { + g_autoptr(virHashTable) tmpvars = NULL; + VIR_DEBUG("Following filter %s", inc->filterref); if (!(obj = virNWFilterObjListFindInstantiateFilter(driver->nwfilters, inc->filterref))) @@ -472,9 +471,6 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, missing_vars, useNewFilter, driver); - - virHashFree(tmpvars); - virNWFilterObjUnlock(obj); if (rc < 0) return -1; @@ -515,7 +511,7 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver, int rc; virNWFilterInst inst; bool instantiate = true; - char *buf; + g_autofree char *buf = NULL; virNWFilterVarValuePtr lv; const char *learning; bool reportIP = false; @@ -635,7 +631,6 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver, virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot instantiate filter due to unresolvable " "variables or unavailable list elements: %s"), buf); - VIR_FREE(buf); } rc = -1; diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 63fac37132..abef0b6457 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -397,7 +397,7 @@ learnIPAddressThread(void *arg) int dhcp_opts_len; char macaddr[VIR_MAC_STRING_BUFLEN]; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *filter = NULL; + g_autofree char *filter = NULL; uint16_t etherType; bool showError = true; enum howDetect howDetected = 0; @@ -622,8 +622,6 @@ learnIPAddressThread(void *arg) } /* while */ cleanup: - VIR_FREE(filter); - if (handle) pcap_close(handle); @@ -633,7 +631,7 @@ learnIPAddressThread(void *arg) sa.len = sizeof(sa.data.inet4); sa.data.inet4.sin_family = AF_INET; sa.data.inet4.sin_addr.s_addr = vmaddr; - char *inetaddr; + g_autofree char *inetaddr = NULL; /* It is necessary to unlock interface here to avoid updateMutex and * interface ordering deadlocks. Otherwise we are going to @@ -656,7 +654,6 @@ learnIPAddressThread(void *arg) req->ifindex); VIR_DEBUG("Result from applying firewall rules on " "%s with IP addr %s : %d", req->binding->portdevname, inetaddr, ret); - VIR_FREE(inetaddr); } } else { if (showError)