[libvirt] [PATCH] virnetdev: Fix Memory Leak in virNetDevReplaceMacAddress() and virNetDevRestoreMacAddress()
Michal Privoznik
mprivozn at redhat.com
Tue Apr 8 09:24:04 UTC 2014
On 08.04.2014 06:25, Wangrui (K) wrote:
> function virNetDevRestoreMacAddress() and virNetDevRestoreMacAddress() alloc memory for
> variable 'path' using virAsprintf(), but they havn't free that memory before returning out.
The subject is rather long.
s/function/Functions/
s/alloc/allocate/
s/havn't free/haven't freed/
>
> Signed-off-by: Zhang bo <oscar.zhangbo at huawei.com>
> ---
> src/util/virnetdev.c | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index f26ea80..853f6ef 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -310,6 +310,7 @@ virNetDevReplaceMacAddress(const char *linkdev,
> virMacAddr oldmac;
> char *path = NULL;
> char macstr[VIR_MAC_STRING_BUFLEN];
> + int ret = -1;
>
> if (virNetDevGetMAC(linkdev, &oldmac) < 0)
> return -1;
> @@ -323,13 +324,17 @@ virNetDevReplaceMacAddress(const char *linkdev,
> if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY) < 0) {
> virReportSystemError(errno, _("Unable to preserve mac for %s"),
> linkdev);
> - return -1;
> + goto cleanup;
Indentation's off.
> }
>
> if (virNetDevSetMAC(linkdev, macaddress) < 0)
> - return -1;
> + goto cleanup;
>
> - return 0;
> + ret = 0;
> +
> +cleanup:
Here we tend to put space prior to the label name..
> + VIR_FREE(path);
> + return ret;
> }
>
> /**
> @@ -344,7 +349,7 @@ int
> virNetDevRestoreMacAddress(const char *linkdev,
> const char *stateDir)
> {
> - int rc;
> + int rc = -1;
> char *oldmacname = NULL;
> char *macstr = NULL;
> char *path = NULL;
> @@ -356,21 +361,22 @@ virNetDevRestoreMacAddress(const char *linkdev,
> return -1;
>
> if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN, &macstr) < 0)
> - return -1;
> + goto cleanup;
>
> if (virMacAddrParse(macstr, &oldmac) != 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Cannot parse MAC address from '%s'"),
> oldmacname);
> - VIR_FREE(macstr);
> - return -1;
> + goto cleanup;
> }
>
> /*reset mac and remove file-ignore results*/
> rc = virNetDevSetMAC(linkdev, &oldmac);
> ignore_value(unlink(path));
> - VIR_FREE(macstr);
>
> +cleanup:
> + VIR_FREE(macstr);
> + VIR_FREE(path);
> return rc;
> }
>
>
ACKed with this squashed in:
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 853f6ef..3a60cf7 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -315,7 +315,6 @@ virNetDevReplaceMacAddress(const char *linkdev,
if (virNetDevGetMAC(linkdev, &oldmac) < 0)
return -1;
-
if (virAsprintf(&path, "%s/%s",
stateDir,
linkdev) < 0)
@@ -324,7 +323,7 @@ virNetDevReplaceMacAddress(const char *linkdev,
if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY) < 0) {
virReportSystemError(errno, _("Unable to preserve mac for %s"),
linkdev);
- goto cleanup;
+ goto cleanup;
}
if (virNetDevSetMAC(linkdev, macaddress) < 0)
@@ -332,7 +331,7 @@ virNetDevReplaceMacAddress(const char *linkdev,
ret = 0;
-cleanup:
+ cleanup:
VIR_FREE(path);
return ret;
}
@@ -374,7 +373,7 @@ virNetDevRestoreMacAddress(const char *linkdev,
rc = virNetDevSetMAC(linkdev, &oldmac);
ignore_value(unlink(path));
-cleanup:
+ cleanup:
VIR_FREE(macstr);
VIR_FREE(path);
return rc;
Adjusted the commit message and pushed. Congratulations to Oscar on his first libvirt patch!
Michal
More information about the libvir-list
mailing list