[libvirt] [PATCH] virbitmaptest: Shut coverity up in case of broken test
Coverity reported a memleak in the test added in 7efd5fd1b02. In case the code will be broken and the code will actually parse a faulty bitmap the resulting pointer would be leaked. Free it although that shouldn't ever happen. --- tests/virbitmaptest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index c56d6fa..e00b0a0 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -492,6 +492,7 @@ test9(const void *opaque ATTRIBUTE_UNUSED) ret = 0; cleanup: + virBitmapFree(bitmap); return ret; } -- 1.8.3.2
On 08/19/2013 08:11 AM, Peter Krempa wrote:
Coverity reported a memleak in the test added in 7efd5fd1b02. In case the code will be broken and the code will actually parse a faulty bitmap the resulting pointer would be leaked. Free it although that shouldn't ever happen. --- tests/virbitmaptest.c | 1 + 1 file changed, 1 insertion(+)
ACK John
On 08/19/2013 06:11 AM, Peter Krempa wrote:
Coverity reported a memleak in the test added in 7efd5fd1b02. In case the code will be broken and the code will actually parse a faulty bitmap the resulting pointer would be leaked. Free it although that shouldn't ever happen. --- tests/virbitmaptest.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index c56d6fa..e00b0a0 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -492,6 +492,7 @@ test9(const void *opaque ATTRIBUTE_UNUSED)
ret = 0; cleanup: + virBitmapFree(bitmap);
Hmm - in this function, bitmap starts life uninitialized, but virBitmapParse starts with 'if (!str) return -1;' prior to '*bitmap = ...'. Therefore, it is possible that Coverity may complain about getting to the cleanup function and freeing uninitialized memory. It may be worth improving virBitmapParse to do '*bitmap = NULL' prior to any return value, so that the caller always has an initialized variable after the call. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/19/13 15:12, Eric Blake wrote:
On 08/19/2013 06:11 AM, Peter Krempa wrote:
Coverity reported a memleak in the test added in 7efd5fd1b02. In case the code will be broken and the code will actually parse a faulty bitmap the resulting pointer would be leaked. Free it although that shouldn't ever happen. --- tests/virbitmaptest.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index c56d6fa..e00b0a0 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -492,6 +492,7 @@ test9(const void *opaque ATTRIBUTE_UNUSED)
ret = 0; cleanup: + virBitmapFree(bitmap);
Hmm - in this function, bitmap starts life uninitialized, but virBitmapParse starts with 'if (!str) return -1;' prior to '*bitmap = ...'. Therefore, it is possible that Coverity may complain about getting to the cleanup function and freeing uninitialized memory.
It may be worth improving virBitmapParse to do '*bitmap = NULL' prior to any return value, so that the caller always has an initialized variable after the call.
Hmm, I didn't notice that when fixing virBitmapParse. I'll push this test with bitmap initialized to NULL and I'll follow up with fixing virBitmapParse. Peter
participants (3)
-
Eric Blake -
John Ferlan -
Peter Krempa