Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
tests/virbitmaptest.c | 73 ++++++++++++++-----------------------------
1 file changed, 24 insertions(+), 49 deletions(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index 1578cd0612..bfca208a7f 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -29,7 +29,7 @@
static int
test1(const void *data G_GNUC_UNUSED)
{
- virBitmapPtr bitmap;
+ g_autoptr(virBitmap) bitmap = NULL;
int size;
int bit;
bool result;
@@ -58,7 +58,6 @@ test1(const void *data G_GNUC_UNUSED)
ret = 0;
error:
- virBitmapFree(bitmap);
return ret;
}
@@ -85,8 +84,8 @@ static int
test2(const void *data G_GNUC_UNUSED)
{
const char *bitsString1 = "1-32,50,88-99,1021-1023";
- char *bitsString2 = NULL;
- virBitmapPtr bitmap = NULL;
+ g_autofree char *bitsString2 = NULL;
+ g_autoptr(virBitmap) bitmap = NULL;
int ret = -1;
int size = 1025;
@@ -139,15 +138,13 @@ test2(const void *data G_GNUC_UNUSED)
ret = 0;
error:
- virBitmapFree(bitmap);
- VIR_FREE(bitsString2);
return ret;
}
static int
test3(const void *data G_GNUC_UNUSED)
{
- virBitmapPtr bitmap = NULL;
+ g_autoptr(virBitmap) bitmap = NULL;
int ret = -1;
int size = 5;
size_t i;
@@ -167,7 +164,6 @@ test3(const void *data G_GNUC_UNUSED)
ret = 0;
error:
- virBitmapFree(bitmap);
return ret;
}
@@ -185,7 +181,7 @@ test4(const void *data G_GNUC_UNUSED)
1, 5, 11, 13, 19, 21, 23, 24, 26, 27,
28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39
};
- virBitmapPtr bitmap = NULL;
+ g_autoptr(virBitmap) bitmap = NULL;
ssize_t i, j;
if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size)
@@ -285,11 +281,9 @@ test4(const void *data G_GNUC_UNUSED)
if (virBitmapNextClearBit(bitmap, -1) != -1)
goto error;
- virBitmapFree(bitmap);
return 0;
error:
- virBitmapFree(bitmap);
return -1;
}
@@ -298,14 +292,14 @@ static int
test5(const void *v G_GNUC_UNUSED)
{
char data[] = {0x01, 0x02, 0x00, 0x00, 0x04};
- unsigned char *data2 = NULL;
+ g_autofree unsigned char *data2 = NULL;
int len2;
int bits[] = {0, 9, 34};
- virBitmapPtr bitmap;
+ g_autoptr(virBitmap) bitmap = NULL;
size_t i;
ssize_t j;
int ret = -1;
- char *str = NULL;
+ g_autofree char *str = NULL;
bitmap = virBitmapNewData(data, sizeof(data));
if (!bitmap)
@@ -347,9 +341,6 @@ test5(const void *v G_GNUC_UNUSED)
ret = 0;
error:
- VIR_FREE(str);
- virBitmapFree(bitmap);
- VIR_FREE(data2);
return ret;
}
@@ -358,8 +349,8 @@ test5(const void *v G_GNUC_UNUSED)
static int
test6(const void *v G_GNUC_UNUSED)
{
- virBitmapPtr bitmap = NULL;
- char *str = NULL;
+ g_autoptr(virBitmap) bitmap = NULL;
+ g_autofree char *str = NULL;
int size = 64;
int ret = -1;
@@ -432,15 +423,12 @@ test6(const void *v G_GNUC_UNUSED)
ret = 0;
error:
- virBitmapFree(bitmap);
- VIR_FREE(str);
return ret;
}
static int
test7(const void *v G_GNUC_UNUSED)
{
- virBitmapPtr bitmap;
size_t i;
size_t maxBit[] = {
1, 8, 31, 32, 63, 64, 95, 96, 127, 128, 159, 160
@@ -448,7 +436,7 @@ test7(const void *v G_GNUC_UNUSED)
size_t nmaxBit = 12;
for (i = 0; i < nmaxBit; i++) {
- bitmap = virBitmapNew(maxBit[i]);
+ g_autoptr(virBitmap) bitmap = virBitmapNew(maxBit[i]);
if (!bitmap)
goto error;
@@ -466,21 +454,18 @@ test7(const void *v G_GNUC_UNUSED)
virBitmapClearAll(bitmap);
if (!virBitmapIsAllClear(bitmap))
goto error;
-
- virBitmapFree(bitmap);
}
return 0;
error:
- virBitmapFree(bitmap);
return -1;
}
static int
test8(const void *v G_GNUC_UNUSED)
{
- virBitmapPtr bitmap = NULL;
+ g_autoptr(virBitmap) bitmap = NULL;
char data[108] = {0x00,};
int ret = -1;
@@ -499,7 +484,6 @@ test8(const void *v G_GNUC_UNUSED)
ret = 0;
cleanup:
- virBitmapFree(bitmap);
return ret;
}
@@ -509,7 +493,7 @@ static int
test9(const void *opaque G_GNUC_UNUSED)
{
int ret = -1;
- virBitmapPtr bitmap = NULL;
+ g_autoptr(virBitmap) bitmap = NULL;
if (virBitmapParse("100000000", &bitmap, 20) != -1)
goto cleanup;
@@ -531,7 +515,6 @@ test9(const void *opaque G_GNUC_UNUSED)
ret = 0;
cleanup:
- virBitmapFree(bitmap);
return ret;
}
@@ -540,7 +523,10 @@ static int
test10(const void *opaque G_GNUC_UNUSED)
{
int ret = -1;
- virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL;
+ g_autoptr(virBitmap) b1 = NULL;
+ g_autoptr(virBitmap) b2 = NULL;
+ g_autoptr(virBitmap) b3 = NULL;
+ g_autoptr(virBitmap) b4 = NULL;
if (virBitmapParseSeparator("0-3,5-8,11-15f16", 'f', &b1, 20) < 0 ||
virBitmapParse("4,9,10,16-19", &b2, 20) < 0 ||
@@ -561,10 +547,6 @@ test10(const void *opaque G_GNUC_UNUSED)
ret = 0;
cleanup:
- virBitmapFree(b1);
- virBitmapFree(b2);
- virBitmapFree(b3);
- virBitmapFree(b4);
return ret;
}
@@ -578,9 +560,9 @@ static int
test11(const void *opaque)
{
const struct testBinaryOpData *data = opaque;
- virBitmapPtr amap = NULL;
- virBitmapPtr bmap = NULL;
- virBitmapPtr resmap = NULL;
+ g_autoptr(virBitmap) amap = NULL;
+ g_autoptr(virBitmap) bmap = NULL;
+ g_autoptr(virBitmap) resmap = NULL;
int ret = -1;
if (virBitmapParse(data->a, &amap, 256) < 0 ||
@@ -600,9 +582,6 @@ test11(const void *opaque)
ret = 0;
cleanup:
- virBitmapFree(amap);
- virBitmapFree(bmap);
- virBitmapFree(resmap);
return ret;
}
@@ -631,7 +610,7 @@ test11(const void *opaque)
static int
test12(const void *opaque G_GNUC_UNUSED)
{
- virBitmapPtr map = virBitmapNewEmpty();
+ g_autoptr(virBitmap) map = virBitmapNewEmpty();
int ret = -1;
TEST_MAP(0, "");
@@ -661,7 +640,6 @@ test12(const void *opaque G_GNUC_UNUSED)
ret = 0;
cleanup:
- virBitmapFree(map);
return ret;
}
@@ -699,9 +677,9 @@ static int
test14(const void *opaque)
{
const struct testBinaryOpData *data = opaque;
- virBitmapPtr amap = NULL;
- virBitmapPtr bmap = NULL;
- virBitmapPtr resmap = NULL;
+ g_autoptr(virBitmap) amap = NULL;
+ g_autoptr(virBitmap) bmap = NULL;
+ g_autoptr(virBitmap) resmap = NULL;
int ret = -1;
if (virBitmapParse(data->a, &amap, 256) < 0 ||
@@ -721,9 +699,6 @@ test14(const void *opaque)
ret = 0;
cleanup:
- virBitmapFree(amap);
- virBitmapFree(bmap);
- virBitmapFree(resmap);
return ret;
}
--
2.26.2
On a Friday in 2020, Peter Krempa wrote:
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> tests/virbitmaptest.c | 73 ++++++++++++++-----------------------------
> 1 file changed, 24 insertions(+), 49 deletions(-)
>
>diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
>index 1578cd0612..bfca208a7f 100644
>--- a/tests/virbitmaptest.c
>+++ b/tests/virbitmaptest.c
>@@ -185,7 +181,7 @@ test4(const void *data G_GNUC_UNUSED)
> 1, 5, 11, 13, 19, 21, 23, 24, 26, 27,
> 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39
> };
>- virBitmapPtr bitmap = NULL;
>+ g_autoptr(virBitmap) bitmap = NULL;
Here, bitmap is also freed in the middle of the function
(it seems these are three independent tests squashed into one function)
> ssize_t i, j;
>
> if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size)
>@@ -285,11 +281,9 @@ test4(const void *data G_GNUC_UNUSED)
> if (virBitmapNextClearBit(bitmap, -1) != -1)
> goto error;
>
>- virBitmapFree(bitmap);
> return 0;
>
> error:
>- virBitmapFree(bitmap);
> return -1;
> }
>
>@@ -298,14 +292,14 @@ static int
> test5(const void *v G_GNUC_UNUSED)
> {
> char data[] = {0x01, 0x02, 0x00, 0x00, 0x04};
>- unsigned char *data2 = NULL;
>+ g_autofree unsigned char *data2 = NULL;
> int len2;
> int bits[] = {0, 9, 34};
>- virBitmapPtr bitmap;
>+ g_autoptr(virBitmap) bitmap = NULL;
> size_t i;
> ssize_t j;
> int ret = -1;
>- char *str = NULL;
>+ g_autofree char *str = NULL;
This one is also freed in the middle.
>
> bitmap = virBitmapNewData(data, sizeof(data));
> if (!bitmap)
>@@ -347,9 +341,6 @@ test5(const void *v G_GNUC_UNUSED)
>
> ret = 0;
> error:
>- VIR_FREE(str);
>- virBitmapFree(bitmap);
>- VIR_FREE(data2);
> return ret;
> }
>
>@@ -358,8 +349,8 @@ test5(const void *v G_GNUC_UNUSED)
> static int
> test6(const void *v G_GNUC_UNUSED)
> {
>- virBitmapPtr bitmap = NULL;
>- char *str = NULL;
>+ g_autoptr(virBitmap) bitmap = NULL;
>+ g_autofree char *str = NULL;
Same here.
> int size = 64;
> int ret = -1;
>
>@@ -631,7 +610,7 @@ test11(const void *opaque)
> static int
> test12(const void *opaque G_GNUC_UNUSED)
> {
>- virBitmapPtr map = virBitmapNewEmpty();
>+ g_autoptr(virBitmap) map = virBitmapNewEmpty();
`map` is freed in the middle of the function.
> int ret = -1;
>
> TEST_MAP(0, "");
>@@ -661,7 +640,6 @@ test12(const void *opaque G_GNUC_UNUSED)
> ret = 0;
>
> cleanup:
>- virBitmapFree(map);
> return ret;
> }
>
Jano
On Fri, Oct 02, 2020 at 10:50:55 +0200, Ján Tomko wrote: > On a Friday in 2020, Peter Krempa wrote: > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > > --- > > tests/virbitmaptest.c | 73 ++++++++++++++----------------------------- > > 1 file changed, 24 insertions(+), 49 deletions(-) > > > > diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c > > index 1578cd0612..bfca208a7f 100644 > > --- a/tests/virbitmaptest.c > > +++ b/tests/virbitmaptest.c > > @@ -185,7 +181,7 @@ test4(const void *data G_GNUC_UNUSED) > > 1, 5, 11, 13, 19, 21, 23, 24, 26, 27, > > 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39 > > }; > > - virBitmapPtr bitmap = NULL; > > + g_autoptr(virBitmap) bitmap = NULL; > > Here, bitmap is also freed in the middle of the function > (it seems these are three independent tests squashed into one function) What's worse, test4 actually doesn't report any errors if any of the cases would fail. You'd have to use a debugger to figure out what's wrong. Note that all of the cases you mention below actually clear the pointer when the contents are freed, so this patch is still correctly handling the memory. Should I post another version where I add separate variables for any reuse?
On a Friday in 2020, Peter Krempa wrote: >On Fri, Oct 02, 2020 at 10:50:55 +0200, Ján Tomko wrote: >> On a Friday in 2020, Peter Krempa wrote: >> > Signed-off-by: Peter Krempa <pkrempa@redhat.com> >> > --- >> > tests/virbitmaptest.c | 73 ++++++++++++++----------------------------- >> > 1 file changed, 24 insertions(+), 49 deletions(-) >> > >> > diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c >> > index 1578cd0612..bfca208a7f 100644 >> > --- a/tests/virbitmaptest.c >> > +++ b/tests/virbitmaptest.c >> > @@ -185,7 +181,7 @@ test4(const void *data G_GNUC_UNUSED) >> > 1, 5, 11, 13, 19, 21, 23, 24, 26, 27, >> > 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39 >> > }; >> > - virBitmapPtr bitmap = NULL; >> > + g_autoptr(virBitmap) bitmap = NULL; >> >> Here, bitmap is also freed in the middle of the function >> (it seems these are three independent tests squashed into one function) > >What's worse, test4 actually doesn't report any errors if any of the >cases would fail. You'd have to use a debugger to figure out what's >wrong. > >Note that all of the cases you mention below actually clear the pointer >when the contents are freed, so this patch is still correctly handling >the memory. > Yes, I do not object to the current handling of the memory - just that mixing the autofree approach with manual freeing can possibly lead us to a change that will disrupt that balance. https://www.redhat.com/archives/libvir-list/2019-July/msg00987.html >Should I post another version where I add separate variables for any >reuse? I was thinking they could be separated into test4a test4b etc, but that works too. Jano
© 2016 - 2026 Red Hat, Inc.