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(a)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