[PATCH 1/4] vsh-table: Use g_autofree where possible
Laine Stump
laine at redhat.com
Wed Mar 3 23:25:33 UTC 2021
On 3/1/21 10:54 AM, Ján Tomko wrote:
> On a Thursday in 2021, Michal Privoznik wrote:
>> On 2/25/21 1:20 PM, Ján Tomko wrote:
>>> On a Tuesday in 2021, Kristina Hanicova wrote:
>>>> In: vshTableRowNew(), vshTablePrint(), vshTablePrintToStdout().
>>>>
>>>> Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
>>>> ---
>>>> tools/vsh-table.c | 16 +++++-----------
>>>> 1 file changed, 5 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/tools/vsh-table.c b/tools/vsh-table.c
>>>> index d09cc9e14e..2e10abfc90 100644
>>>> --- a/tools/vsh-table.c
>>>> +++ b/tools/vsh-table.c
>>>> @@ -361,8 +359,8 @@ vshTablePrint(vshTablePtr table, bool header)
>>>> {
>>>> size_t i;
>>>> size_t j;
>>>> - size_t *maxwidths;
>>>> - size_t **widths;
>>>> + g_autofree size_t *maxwidths = NULL;
>>>> + g_autofree size_t **widths = NULL;
>>>> g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>>>> char *ret = NULL;
>>>>
>>>> @@ -395,10 +393,8 @@ vshTablePrint(vshTablePtr table, bool header)
>>>> ret = virBufferContentAndReset(&buf);
>>>>
>>>> cleanup:
>>>> - VIR_FREE(maxwidths);
>>>
>>>> for (i = 0; i < table->nrows; i++)
>>>> VIR_FREE(widths[i]);
>>>> - VIR_FREE(widths);
>>>
>>> While this does not change the behavior, mixing g_autofree for the outer
>>> array while using VIR_FREE for the per-row arrays feels incomplete.
>>
>> Any idea what would make it feel complete again?
>>
>
> Freeing all the memory associated with 'widths' automatically, or none
> of it.
Yeah, I think that arrays of pointers to allocated objects were the only
uses of VIR_FREE that I ended up not eliminating in my patches that
removed *most* VIR_FREE from the esx directory. I didn't have the
motivation to figure out the proper best way to fix them...
>
> So either:
> * introduce a new typedef for 'size_t **' and define a cleanup function
> for it that does that
> * use a different data type (does GLib have one that could do this for
> us?)
glib has GArray (arrays of arbitrarily-sized elements) and GPtrArray
(arrays of pointers). I suppose we should be using those (although they
seem more complicated than VIR_INSERT_ELEMENT and friends, I'm probably
unfairly biased. (I do acknowledge that it's nice that they have the
size as a part of the array).
>
> But both seem out of scope of a simple g_autofree cleanup.
Yeah, it's going to take some of that stuff, what's it called now?....
Oh yeah - initiative, that's what it is.
(I'm actually just about to try fixing a bug (not related to memory
management) using GArray for the first time. I'll let you know how it goes)
More information about the libvir-list
mailing list