[libvirt] [PATCH] tests: Fix memory leak in virnetmessagetest

Detected when playing with "make -C tests valgrind". --- tests/virnetmessagetest.c | 35 ++++++++++++++++++++++++----------- 1 files changed, 24 insertions(+), 11 deletions(-) diff --git a/tests/virnetmessagetest.c b/tests/virnetmessagetest.c index e707b67..61f457c 100644 --- a/tests/virnetmessagetest.c +++ b/tests/virnetmessagetest.c @@ -167,6 +167,7 @@ static int testMessagePayloadEncode(const void *args ATTRIBUTE_UNUSED) { virNetMessageError err; static virNetMessage msg; + int ret = -1; static const char expect[] = { 0x00, 0x00, 0x00, 0x74, /* Length */ 0x11, 0x22, 0x33, 0x44, /* Program */ @@ -204,19 +205,21 @@ static int testMessagePayloadEncode(const void *args ATTRIBUTE_UNUSED) err.code = VIR_ERR_INTERNAL_ERROR; err.domain = VIR_FROM_RPC; + err.level = VIR_ERR_ERROR; + if (VIR_ALLOC(err.message) < 0) - return -1; + goto cleanup; *err.message = strdup("Hello World"); - err.level = VIR_ERR_ERROR; if (VIR_ALLOC(err.str1) < 0) - return -1; + goto cleanup; *err.str1 = strdup("One"); if (VIR_ALLOC(err.str2) < 0) - return -1; + goto cleanup; *err.str2 = strdup("Two"); if (VIR_ALLOC(err.str3) < 0) - return -1; + goto cleanup; *err.str3 = strdup("Three"); + err.int1 = 1; err.int2 = 2; @@ -228,29 +231,39 @@ static int testMessagePayloadEncode(const void *args ATTRIBUTE_UNUSED) msg.header.status = VIR_NET_ERROR; if (virNetMessageEncodeHeader(&msg) < 0) - return -1; + goto cleanup; if (virNetMessageEncodePayload(&msg, (xdrproc_t)xdr_virNetMessageError, &err) < 0) - return -1; + goto cleanup; if (ARRAY_CARDINALITY(expect) != msg.bufferLength) { VIR_DEBUG("Expect message length %zu got %zu", sizeof(expect), msg.bufferLength); - return -1; + goto cleanup; } if (msg.bufferOffset != 0) { VIR_DEBUG("Expect message offset 0 got %zu", msg.bufferOffset); - return -1; + goto cleanup; } if (memcmp(expect, msg.buffer, sizeof(expect)) != 0) { virtTestDifferenceBin(stderr, expect, msg.buffer, sizeof(expect)); - return -1; - } - - return 0; + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(*err.message); + VIR_FREE(*err.str1); + VIR_FREE(*err.str2); + VIR_FREE(*err.str3); + VIR_FREE(err.message); + VIR_FREE(err.str1); + VIR_FREE(err.str2); + VIR_FREE(err.str3); + return ret; } static int testMessagePayloadDecode(const void *args ATTRIBUTE_UNUSED) -- 1.7.1

On 06/28/2011 03:16 AM, Osier Yang wrote:
Detected when playing with "make -C tests valgrind". --- tests/virnetmessagetest.c | 35 ++++++++++++++++++++++++----------- 1 files changed, 24 insertions(+), 11 deletions(-)
if (VIR_ALLOC(err.str3) < 0) - return -1; + goto cleanup; *err.str3 = strdup("Three");
+cleanup: + VIR_FREE(*err.message); + VIR_FREE(*err.str1); + VIR_FREE(*err.str2); + VIR_FREE(*err.str3);
Ouch - four potential NULL derefs. You need to write these as: if (err.str3) VIR_FREE(*err.str3); and so forth.
+ VIR_FREE(err.message); + VIR_FREE(err.str1); + VIR_FREE(err.str2); + VIR_FREE(err.str3);
But these four are okay.
+ return ret;
ACK with that fix. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

于 2011年06月28日 21:10, Eric Blake 写道:
On 06/28/2011 03:16 AM, Osier Yang wrote:
Detected when playing with "make -C tests valgrind". --- tests/virnetmessagetest.c | 35 ++++++++++++++++++++++++----------- 1 files changed, 24 insertions(+), 11 deletions(-)
if (VIR_ALLOC(err.str3)< 0) - return -1; + goto cleanup; *err.str3 = strdup("Three");
+cleanup: + VIR_FREE(*err.message); + VIR_FREE(*err.str1); + VIR_FREE(*err.str2); + VIR_FREE(*err.str3);
Ouch - four potential NULL derefs. You need to write these as:
if (err.str3) VIR_FREE(*err.str3);
and so forth.
ah, yes
+ VIR_FREE(err.message); + VIR_FREE(err.str1); + VIR_FREE(err.str2); + VIR_FREE(err.str3);
But these four are okay.
+ return ret;
ACK with that fix.
Applied with the fix, thanks Osier
participants (2)
-
Eric Blake
-
Osier Yang