Re: [PATCH 1/4] vsh-table: Use g_autofree where possible

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
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)




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux