Re: [libvirt] [PATCH 1/2] Add API for printing tables.

[Simon Kobyda]

On Mon, 2018-08-13 at 12:01 +0200, Michal Prívozník wrote:
> On 08/13/2018 11:46 AM, Simon Kobyda wrote:
> > On Fri, 2018-08-10 at 15:40 +0200, Michal Privoznik wrote:
> > > On 08/10/2018 11:11 AM, Simon Kobyda wrote:
> 
> 
> > > > +static vshTableRowPtr
> > > > +vshTableRowNew(size_t size, const char *arg, va_list ap)
> > > 
> > > I know we discussed this in person, but now that I see this code
> > > and
> > > think about it more I think this @size argument should be dropped
> > > from
> > > this function and the corresponding check should be moved to [1]
> > 
> > That way I will implement identical loop also there [1], to count
> > number of variable arguments to make that check (if there is better
> > way
> > to count number of variable arguments, I'm open to suggestions :)
> > ).
> > Sure I can drop it there, but what are pros of that? I'm curious
> > :).
> 
> I'm not sure why you would need the second loop? The way your code
> works
> right now is that vshTableRowNew must have the sentinel (= args have
> to
> end with NULL). What I am suggesting is for vshTableRowNew() to just
> consume all the args (like it is doing now) until the sentinel. And
> only
> after that check at [1]  whether row->ncells is expected value.
> 
I see, I misunderstood. That's also an option.
> > > 
> > > > +{
> > > > +vshTableRowPtr row = NULL;
> > > > +char *tmp = NULL;
> > > > +
> > > > +if (!arg) {
> > > > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > +_("Table row cannot be empty"));
> > > > +goto error;
> > > > +}
> > > > +
> > > > +if (VIR_ALLOC(row) < 0)
> > > > +goto error;
> > > > +
> > > > +while (arg) {
> > > > +if (VIR_STRDUP(tmp, arg) < 0)
> > > > +goto error;
> > > > +
> > > > +if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) <
> > > > 0)
> > > > +goto error;
> > > > +
> > > > +arg = va_arg(ap, const char *);
> > > > +}
> > > > +
> > > > +if (size && row->ncells != size) {
> > > > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > +_("Incorrect number of cells in a
> > > > table
> > > > row"));
> > > > +goto error;
> > > > +}
> > > > +
> > > > +return row;
> > > > +
> > > > + error:
> > > > +vshTableRowFree(row);
> > > > +return NULL;
> > > > +}
> > > > +
> 
> 
> > > > +
> > > > +/**
> > > > + * vshTableRowPrint:
> > > > + * @ctl virtshell control structure
> > > > + * @row: table to append to
> > > > + * @maxwidths: maximum count of characters for each columns
> > > > + * @widths: count of character for each cell in this row
> > > > + * @printCB function for priting table
> > > > + * @buf: buffer to store table (only if @toStdout == true)
> > > > + * @toStdout: whetever print table to Stdout or return in
> > > > buffer
> > > > + */
> > > > +static void
> > > > +vshTableRowPrint(vshControl *ctl,
> > > > + vshTableRowPtr row,
> > > > + size_t *maxwidths,
> > > > + size_t *widths,
> > > > + vshPrintCB printCB,
> > > > + virBufferPtr buf,
> > > > + bool toStdout)
> > > > +{
> > > > +size_t i;
> > > > +size_t j;
> > > > +
> > > > +if (toStdout) {
> > > > +for (i = 0; i < row->ncells; i++) {
> > > > +printCB(ctl, " %s", row->cells[i]);
> > > > +
> > > > +for (j = 0; j < maxwidths[i] - widths[i] + 2; j++)
> > > > +printCB(ctl, " ");
> > > > +}
> > > > +printCB(ctl, "\n");
> > > > +} else {
> > > > +for (i = 0; i < row->ncells; i++) {
> > > > +virBufferAsprintf(buf, " %s", row->cells[i]);
> > > > +
> > > > +for (j = 0; j < maxwidths[i] - widths[i] + 2; j++)
> > > > +virBufferAddStr(buf, " ");
> > > > +}
> > > > +virBufferAddStr(buf, "\n");
> > > > +}
> > > 
> > > 
> > > How about inverting these ifs and fors? I mean:
> > > 
> > > for () {
> > >   if (toStdout)
> > > printCB()
> > >   else
> > > virBufferAsprintf.
> > > }
> > > 
> > > This suggestion applies to vshTablePrint too.
> > 
> > I had it the way you are suggesting before, but I inverted it
> > because I
> > think the current state of code is more readable, compared to its
> > inverted state. 
> > The length of code would be very similar, but perhaps it would be
> > more
> > optimized? (we would have only 2 'for' loops instead of 4, however
> > more
> > if-else statements) If that's the reason why are you suggesting to
> > invert it.
> 
> Well, the reason is that if we ever need to change the way we're
> iterating (e.g. call some function at the end of each loop) we can do
> it
> in one change:
> 
>  for () {
>if (toStdout)
>  printCB()
>else
>  virBufferAsprintf()
> 
> +  newFunction();
>  }
> 
> 
> Michal
> > 
> > > 
> > > > +
> > > > +}
> > > > +
> > > > +/**
> > > > + * vshTablePrint:
> > > > + * 

Re: [libvirt] [PATCH 1/2] Add API for printing tables.

[Michal Prívozník]

On 08/13/2018 11:46 AM, Simon Kobyda wrote:
> On Fri, 2018-08-10 at 15:40 +0200, Michal Privoznik wrote:
>> On 08/10/2018 11:11 AM, Simon Kobyda wrote:


>>> +static vshTableRowPtr
>>> +vshTableRowNew(size_t size, const char *arg, va_list ap)
>>
>> I know we discussed this in person, but now that I see this code and
>> think about it more I think this @size argument should be dropped
>> from
>> this function and the corresponding check should be moved to [1]
> That way I will implement identical loop also there [1], to count
> number of variable arguments to make that check (if there is better way
> to count number of variable arguments, I'm open to suggestions :) ).
> Sure I can drop it there, but what are pros of that? I'm curious :).

I'm not sure why you would need the second loop? The way your code works
right now is that vshTableRowNew must have the sentinel (= args have to
end with NULL). What I am suggesting is for vshTableRowNew() to just
consume all the args (like it is doing now) until the sentinel. And only
after that check at [1]  whether row->ncells is expected value.

>>
>>> +{
>>> +vshTableRowPtr row = NULL;
>>> +char *tmp = NULL;
>>> +
>>> +if (!arg) {
>>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +_("Table row cannot be empty"));
>>> +goto error;
>>> +}
>>> +
>>> +if (VIR_ALLOC(row) < 0)
>>> +goto error;
>>> +
>>> +while (arg) {
>>> +if (VIR_STRDUP(tmp, arg) < 0)
>>> +goto error;
>>> +
>>> +if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0)
>>> +goto error;
>>> +
>>> +arg = va_arg(ap, const char *);
>>> +}
>>> +
>>> +if (size && row->ncells != size) {
>>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +_("Incorrect number of cells in a table
>>> row"));
>>> +goto error;
>>> +}
>>> +
>>> +return row;
>>> +
>>> + error:
>>> +vshTableRowFree(row);
>>> +return NULL;
>>> +}
>>> +


>>> +
>>> +/**
>>> + * vshTableRowPrint:
>>> + * @ctl virtshell control structure
>>> + * @row: table to append to
>>> + * @maxwidths: maximum count of characters for each columns
>>> + * @widths: count of character for each cell in this row
>>> + * @printCB function for priting table
>>> + * @buf: buffer to store table (only if @toStdout == true)
>>> + * @toStdout: whetever print table to Stdout or return in buffer
>>> + */
>>> +static void
>>> +vshTableRowPrint(vshControl *ctl,
>>> + vshTableRowPtr row,
>>> + size_t *maxwidths,
>>> + size_t *widths,
>>> + vshPrintCB printCB,
>>> + virBufferPtr buf,
>>> + bool toStdout)
>>> +{
>>> +size_t i;
>>> +size_t j;
>>> +
>>> +if (toStdout) {
>>> +for (i = 0; i < row->ncells; i++) {
>>> +printCB(ctl, " %s", row->cells[i]);
>>> +
>>> +for (j = 0; j < maxwidths[i] - widths[i] + 2; j++)
>>> +printCB(ctl, " ");
>>> +}
>>> +printCB(ctl, "\n");
>>> +} else {
>>> +for (i = 0; i < row->ncells; i++) {
>>> +virBufferAsprintf(buf, " %s", row->cells[i]);
>>> +
>>> +for (j = 0; j < maxwidths[i] - widths[i] + 2; j++)
>>> +virBufferAddStr(buf, " ");
>>> +}
>>> +virBufferAddStr(buf, "\n");
>>> +}
>>
>>
>> How about inverting these ifs and fors? I mean:
>>
>> for () {
>>   if (toStdout)
>> printCB()
>>   else
>> virBufferAsprintf.
>> }
>>
>> This suggestion applies to vshTablePrint too.
> 
> I had it the way you are suggesting before, but I inverted it because I
> think the current state of code is more readable, compared to its
> inverted state. 
> The length of code would be very similar, but perhaps it would be more
> optimized? (we would have only 2 'for' loops instead of 4, however more
> if-else statements) If that's the reason why are you suggesting to
> invert it.

Well, the reason is that if we ever need to change the way we're
iterating (e.g. call some function at the end of each loop) we can do it
in one change:

 for () {
   if (toStdout)
 printCB()
   else
 virBufferAsprintf()

+  newFunction();
 }


Michal
> 
>>
>>> +
>>> +}
>>> +
>>> +/**
>>> + * vshTablePrint:
>>> + * @ctl virtshell control structure
>>> + * @table: table to print
>>> + * @toStdout: whetever to print to stdout (true) or return in
>>> string (false)
>>> + *
>>> + * Prints table. To get an alignment of columns right, function
>>> + * fills 2d array @widths with count of characters in each cell
>>> and
>>> + * array @maxwidths maximum count of character in each column.
>>> + * Function then prints tables header and content.
>>> + *
>>> + * Returns string containing table, or NULL if table was printed
>>> to
>>> + * stdout
>>> + */
>>> +static char *
>>> +vshTablePrint(vshControl *ctl, vshTablePtr table, bool toStdout)
>>> +{
>>> +size_t i;
>>> +

Re: [libvirt] [PATCH 1/2] Add API for printing tables.

[Simon Kobyda]

On Fri, 2018-08-10 at 15:40 +0200, Michal Privoznik wrote:
> On 08/10/2018 11:11 AM, Simon Kobyda wrote:
> > It solves problems with alignment of columns. Width of each column
> > is calculated by its biggest cell. Should solve unicode bug.
> > In future, it may be implemented in virsh, virt-admin...
> > 
> > This API has 5 public functions:
> > - vshTableNew - adds new table and defines its header
> > - vshTableRowAppend - appends new row (for same number of columns
> > as in
> > header)
> > - vshTablePrintToStdout
> > - vshTablePrintToString
> > - vshTableFree
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1574624
> > https://bugzilla.redhat.com/show_bug.cgi?id=1584630
> > 
> > Signed-off-by: Simon Kobyda 
> > ---
> >  tools/Makefile.am |   4 +-
> >  tools/vsh-table.c | 367
> > ++
> >  tools/vsh-table.h |  42 ++
> >  3 files changed, 412 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/vsh-table.c
> >  create mode 100644 tools/vsh-table.h
> > 
> 
> Missing cover letter. When sending more than one patch it's
> necessary.
> Also the patch should have "vsh: " prefix so that it's obvious what
> part
> of the ever growing source it's touching.
> 
> > diff --git a/tools/Makefile.am b/tools/Makefile.am
> > index 26c887649e..ed23575aa7 100644
> > --- a/tools/Makefile.am
> > +++ b/tools/Makefile.am
> > @@ -144,7 +144,9 @@ libvirt_shell_la_LIBADD = \
> > $(READLINE_LIBS) \
> > ../gnulib/lib/libgnu.la \
> > $(NULL)
> > -libvirt_shell_la_SOURCES = vsh.c vsh.h
> > +libvirt_shell_la_SOURCES = \
> > +   vsh.c vsh.h \
> > +   vsh-table.c vsh-table.h
> >  
> >  virt_host_validate_SOURCES = \
> > virt-host-validate.c \
> > diff --git a/tools/vsh-table.c b/tools/vsh-table.c
> > new file mode 100644
> > index 00..11dc807ba9
> > --- /dev/null
> > +++ b/tools/vsh-table.c
> > @@ -0,0 +1,367 @@
> > +/*
> > + * vsh-table.c: table printing helper
> > + *
> > + * Copyright (C) 2018 Red Hat, Inc.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later
> > version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General
> > Public
> > + * License along with this library.  If not, see
> > + * .
> > + *
> > + * Authors:
> > + *   Simon Kobyda 
> > + *
> > + */
> > +
> > +#include 
> > +#include "vsh-table.h"
> > +#include "virsh-util.h"
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "viralloc.h"
> > +#include "virbuffer.h"
> > +#include "virstring.h"
> 
> Nit pick, we tend to include system headers first and only after that
> the local ones.
> 
> > +
> > +typedef void (*vshPrintCB)(vshControl *ctl, const char *fmt, ...);
> > +
> > +struct _vshTableRow {
> > +char **cells;
> > +size_t ncells;
> > +};
> > +
> > +struct _vshTable {
> > +vshTableRowPtr *rows;
> > +size_t nrows;
> > +};
> > +
> > +static void
> > +vshTableRowFree(vshTableRowPtr row)
> > +{
> > +size_t i;
> > +
> > +if (!row)
> > +return;
> > +
> > +for (i = 0; i < row->ncells; i++)
> > +VIR_FREE(row->cells[i]);
> > +
> > +VIR_FREE(row->cells);
> > +VIR_FREE(row);
> > +}
> > +
> > +void
> > +vshTableFree(vshTablePtr table)
> > +{
> > +size_t i;
> > +
> > +if (!table)
> > +return;
> > +
> > +for (i = 0; i < table->nrows; i++)
> > +vshTableRowFree(table->rows[i]);
> > +VIR_FREE(table->rows);
> > +VIR_FREE(table);
> > +}
> > +
> > +/**
> > + * vshTableRowNew:
> > + * @size: expected number of cells in row.
> > + * @arg: the first argument.
> > + * @ap: list of variadic arguments
> > + *
> > + * Creates a new row in the table. Each argument passed
> > + * represents a cell in the row. If the number of cells is not
> > + * equal to @size, then NULL is returned.  However, if @size is
> > + * equal to 0, the aforementioned check is suppressed.
> > + *
> > + * Returns: pointer to vshTableRowPtr row or NULL.
> > + */
> > +static vshTableRowPtr
> > +vshTableRowNew(size_t size, const char *arg, va_list ap)
> 
> I know we discussed this in person, but now that I see this code and
> think about it more I think this @size argument should be dropped
> from
> this function and the corresponding check should be moved to [1]
That way I will implement identical loop also there [1], to count
number of variable arguments to make that check (if there is better way
to count number of variable arguments, 

Re: [libvirt] [PATCH 1/2] Add API for printing tables.

[Michal Privoznik]

On 08/10/2018 11:11 AM, Simon Kobyda wrote:
> It solves problems with alignment of columns. Width of each column
> is calculated by its biggest cell. Should solve unicode bug.
> In future, it may be implemented in virsh, virt-admin...
> 
> This API has 5 public functions:
> - vshTableNew - adds new table and defines its header
> - vshTableRowAppend - appends new row (for same number of columns as in
> header)
> - vshTablePrintToStdout
> - vshTablePrintToString
> - vshTableFree
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1574624
> https://bugzilla.redhat.com/show_bug.cgi?id=1584630
> 
> Signed-off-by: Simon Kobyda 
> ---
>  tools/Makefile.am |   4 +-
>  tools/vsh-table.c | 367 ++
>  tools/vsh-table.h |  42 ++
>  3 files changed, 412 insertions(+), 1 deletion(-)
>  create mode 100644 tools/vsh-table.c
>  create mode 100644 tools/vsh-table.h
> 

Missing cover letter. When sending more than one patch it's necessary.
Also the patch should have "vsh: " prefix so that it's obvious what part
of the ever growing source it's touching.

> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 26c887649e..ed23575aa7 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -144,7 +144,9 @@ libvirt_shell_la_LIBADD = \
>   $(READLINE_LIBS) \
>   ../gnulib/lib/libgnu.la \
>   $(NULL)
> -libvirt_shell_la_SOURCES = vsh.c vsh.h
> +libvirt_shell_la_SOURCES = \
> + vsh.c vsh.h \
> + vsh-table.c vsh-table.h
>  
>  virt_host_validate_SOURCES = \
>   virt-host-validate.c \
> diff --git a/tools/vsh-table.c b/tools/vsh-table.c
> new file mode 100644
> index 00..11dc807ba9
> --- /dev/null
> +++ b/tools/vsh-table.c
> @@ -0,0 +1,367 @@
> +/*
> + * vsh-table.c: table printing helper
> + *
> + * Copyright (C) 2018 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * .
> + *
> + * Authors:
> + *   Simon Kobyda 
> + *
> + */
> +
> +#include 
> +#include "vsh-table.h"
> +#include "virsh-util.h"
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "viralloc.h"
> +#include "virbuffer.h"
> +#include "virstring.h"

Nit pick, we tend to include system headers first and only after that
the local ones.

> +
> +typedef void (*vshPrintCB)(vshControl *ctl, const char *fmt, ...);
> +
> +struct _vshTableRow {
> +char **cells;
> +size_t ncells;
> +};
> +
> +struct _vshTable {
> +vshTableRowPtr *rows;
> +size_t nrows;
> +};
> +
> +static void
> +vshTableRowFree(vshTableRowPtr row)
> +{
> +size_t i;
> +
> +if (!row)
> +return;
> +
> +for (i = 0; i < row->ncells; i++)
> +VIR_FREE(row->cells[i]);
> +
> +VIR_FREE(row->cells);
> +VIR_FREE(row);
> +}
> +
> +void
> +vshTableFree(vshTablePtr table)
> +{
> +size_t i;
> +
> +if (!table)
> +return;
> +
> +for (i = 0; i < table->nrows; i++)
> +vshTableRowFree(table->rows[i]);
> +VIR_FREE(table->rows);
> +VIR_FREE(table);
> +}
> +
> +/**
> + * vshTableRowNew:
> + * @size: expected number of cells in row.
> + * @arg: the first argument.
> + * @ap: list of variadic arguments
> + *
> + * Creates a new row in the table. Each argument passed
> + * represents a cell in the row. If the number of cells is not
> + * equal to @size, then NULL is returned.  However, if @size is
> + * equal to 0, the aforementioned check is suppressed.
> + *
> + * Returns: pointer to vshTableRowPtr row or NULL.
> + */
> +static vshTableRowPtr
> +vshTableRowNew(size_t size, const char *arg, va_list ap)

I know we discussed this in person, but now that I see this code and
think about it more I think this @size argument should be dropped from
this function and the corresponding check should be moved to [1]

> +{
> +vshTableRowPtr row = NULL;
> +char *tmp = NULL;
> +
> +if (!arg) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +_("Table row cannot be empty"));
> +goto error;
> +}
> +
> +if (VIR_ALLOC(row) < 0)
> +goto error;
> +
> +while (arg) {
> +if (VIR_STRDUP(tmp, arg) < 0)
> +goto error;
> +
> +if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0)
> +goto error;
> +
> +arg = va_arg(ap, const char *);
> +}
> +
> +

[libvirt] [PATCH 1/2] Add API for printing tables.

[Simon Kobyda]

It solves problems with alignment of columns. Width of each column
is calculated by its biggest cell. Should solve unicode bug.
In future, it may be implemented in virsh, virt-admin...

This API has 5 public functions:
- vshTableNew - adds new table and defines its header
- vshTableRowAppend - appends new row (for same number of columns as in
header)
- vshTablePrintToStdout
- vshTablePrintToString
- vshTableFree

https://bugzilla.redhat.com/show_bug.cgi?id=1574624
https://bugzilla.redhat.com/show_bug.cgi?id=1584630

Signed-off-by: Simon Kobyda 
---
 tools/Makefile.am |   4 +-
 tools/vsh-table.c | 367 ++
 tools/vsh-table.h |  42 ++
 3 files changed, 412 insertions(+), 1 deletion(-)
 create mode 100644 tools/vsh-table.c
 create mode 100644 tools/vsh-table.h

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 26c887649e..ed23575aa7 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -144,7 +144,9 @@ libvirt_shell_la_LIBADD = \
$(READLINE_LIBS) \
../gnulib/lib/libgnu.la \
$(NULL)
-libvirt_shell_la_SOURCES = vsh.c vsh.h
+libvirt_shell_la_SOURCES = \
+   vsh.c vsh.h \
+   vsh-table.c vsh-table.h
 
 virt_host_validate_SOURCES = \
virt-host-validate.c \
diff --git a/tools/vsh-table.c b/tools/vsh-table.c
new file mode 100644
index 00..11dc807ba9
--- /dev/null
+++ b/tools/vsh-table.c
@@ -0,0 +1,367 @@
+/*
+ * vsh-table.c: table printing helper
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Authors:
+ *   Simon Kobyda 
+ *
+ */
+
+#include 
+#include "vsh-table.h"
+#include "virsh-util.h"
+
+#include 
+#include 
+#include 
+
+#include "viralloc.h"
+#include "virbuffer.h"
+#include "virstring.h"
+
+typedef void (*vshPrintCB)(vshControl *ctl, const char *fmt, ...);
+
+struct _vshTableRow {
+char **cells;
+size_t ncells;
+};
+
+struct _vshTable {
+vshTableRowPtr *rows;
+size_t nrows;
+};
+
+static void
+vshTableRowFree(vshTableRowPtr row)
+{
+size_t i;
+
+if (!row)
+return;
+
+for (i = 0; i < row->ncells; i++)
+VIR_FREE(row->cells[i]);
+
+VIR_FREE(row->cells);
+VIR_FREE(row);
+}
+
+void
+vshTableFree(vshTablePtr table)
+{
+size_t i;
+
+if (!table)
+return;
+
+for (i = 0; i < table->nrows; i++)
+vshTableRowFree(table->rows[i]);
+VIR_FREE(table->rows);
+VIR_FREE(table);
+}
+
+/**
+ * vshTableRowNew:
+ * @size: expected number of cells in row.
+ * @arg: the first argument.
+ * @ap: list of variadic arguments
+ *
+ * Creates a new row in the table. Each argument passed
+ * represents a cell in the row. If the number of cells is not
+ * equal to @size, then NULL is returned.  However, if @size is
+ * equal to 0, the aforementioned check is suppressed.
+ *
+ * Returns: pointer to vshTableRowPtr row or NULL.
+ */
+static vshTableRowPtr
+vshTableRowNew(size_t size, const char *arg, va_list ap)
+{
+vshTableRowPtr row = NULL;
+char *tmp = NULL;
+
+if (!arg) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Table row cannot be empty"));
+goto error;
+}
+
+if (VIR_ALLOC(row) < 0)
+goto error;
+
+while (arg) {
+if (VIR_STRDUP(tmp, arg) < 0)
+goto error;
+
+if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0)
+goto error;
+
+arg = va_arg(ap, const char *);
+}
+
+if (size && row->ncells != size) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Incorrect number of cells in a table row"));
+goto error;
+}
+
+return row;
+
+ error:
+vshTableRowFree(row);
+return NULL;
+}
+
+/**
+ * vshTableNew:
+ * @arg: List of column names (NULL terminated)
+ *
+ * Creates a new table.
+ *
+ * Returns: pointer to table or NULL.
+ */
+vshTablePtr
+vshTableNew(const char *arg, ...)
+{
+vshTablePtr table;
+vshTableRowPtr header = NULL;
+va_list ap;
+
+if (VIR_ALLOC(table) < 0)
+goto error;
+
+va_start(ap, arg);
+header = vshTableRowNew(0, arg, ap);
+va_end(ap);
+
+if (!header)
+goto error;
+
+if (VIR_APPEND_ELEMENT(table->rows, table->nrows, header) < 0)
+goto error;
+
+return table;