This commit fix a wrong variable initialization. There is a variable
called `new_lease` which is being initialized with the content of
parameter `lease`. To avoid memory leak, the proper way is initialize
with NULL first. This wrong statement was added by commit 97a0aa24.
There are some other improvements also.
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
src/conf/network_conf.c | 38 ++++++++++++++++++-------------------
src/network/bridge_driver.c | 8 ++------
2 files changed, 21 insertions(+), 25 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index cd60ee7548..a7c177f8db 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -412,38 +412,32 @@ static int
virNetworkDHCPLeaseTimeDefParseXML(virNetworkDHCPLeaseTimeDefPtr *lease,
xmlNodePtr node)
{
- virNetworkDHCPLeaseTimeDefPtr new_lease = *lease;
- g_autofree char *expiry = NULL;
- g_autofree char *unit = NULL;
- int unitInt;
+ virNetworkDHCPLeaseTimeDefPtr new_lease = NULL;
+ g_autofree char *expirystr = NULL;
+ g_autofree char *unitstr = NULL;
+ unsigned long expiry;
+ int unit = VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES;
- if (!(expiry = virXMLPropString(node, "expiry")))
+ if (!(expirystr = virXMLPropString(node, "expiry")))
return 0;
- if (VIR_ALLOC(new_lease) < 0)
- return -1;
- new_lease->unit = VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES;
-
- if (virStrToLong_ul(expiry, NULL, 10, &new_lease->expiry) < 0)
+ if (virStrToLong_ul(expirystr, NULL, 10, &expiry) < 0)
return -1;
- if ((unit = virXMLPropString(node, "unit"))) {
- if ((unitInt = virNetworkDHCPLeaseTimeUnitTypeFromString(unit)) < 0) {
+ if ((unitstr = virXMLPropString(node, "unit"))) {
+ if ((unit = virNetworkDHCPLeaseTimeUnitTypeFromString(unitstr)) < 0) {
virReportError(VIR_ERR_XML_ERROR,
- _("Invalid unit: %s"), unit);
+ _("Invalid unit: %s"), unitstr);
return -1;
}
- new_lease->unit = unitInt;
}
/* infinite */
- if (new_lease->expiry > 0) {
+ if (expiry > 0) {
/* This boundary check is related to dnsmasq man page settings:
* "The minimum lease time is two minutes." */
- if ((new_lease->unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS &&
- new_lease->expiry < 120) ||
- (new_lease->unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES &&
- new_lease->expiry < 2)) {
+ if ((unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS && expiry < 120)
||
+ (unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES && expiry < 2))
{
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("The minimum lease time should be greater "
"than 2 minutes"));
@@ -451,6 +445,12 @@ virNetworkDHCPLeaseTimeDefParseXML(virNetworkDHCPLeaseTimeDefPtr
*lease,
}
}
+ if (VIR_ALLOC(new_lease) < 0)
+ return -1;
+
+ new_lease->expiry = expiry;
+ new_lease->unit = unit;
+
*lease = new_lease;
return 0;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 87f0452611..e8f0dcf7d0 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -969,7 +969,6 @@ static int networkConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED)
static char *
networkBuildDnsmasqLeaseTime(virNetworkDHCPLeaseTimeDefPtr lease)
{
- char *leasetime = NULL;
const char *unit;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
@@ -984,9 +983,7 @@ networkBuildDnsmasqLeaseTime(virNetworkDHCPLeaseTimeDefPtr lease)
virBufferAsprintf(&buf, "%lu%c", lease->expiry, unit[0]);
}
- leasetime = virBufferContentAndReset(&buf);
-
- return leasetime;
+ return virBufferContentAndReset(&buf);
}
@@ -999,14 +996,13 @@ networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
{
size_t i;
bool ipv6 = false;
- g_autofree char *leasetime = NULL;
if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6))
ipv6 = true;
for (i = 0; i < ipdef->nhosts; i++) {
virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
+ g_autofree char *leasetime = networkBuildDnsmasqLeaseTime(host->lease);
- leasetime = networkBuildDnsmasqLeaseTime(host->lease);
if (VIR_SOCKET_ADDR_VALID(&host->ip))
if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip,
host->name, host->id, leasetime,
--
2.25.3
Show replies by date
On 4/25/20 6:35 PM, Julio Faracco wrote:
This commit fix a wrong variable initialization. There is a variable
called `new_lease` which is being initialized with the content of
parameter `lease`. To avoid memory leak, the proper way is initialize
with NULL first. This wrong statement was added by commit 97a0aa24.
There are some other improvements also.
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
src/conf/network_conf.c | 38 ++++++++++++++++++-------------------
src/network/bridge_driver.c | 8 ++------
2 files changed, 21 insertions(+), 25 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
and pushed.
Michal