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@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