[PATCH 0/4] vsh-table: Various small improvements
*** BLURB HERE *** Michal Prívozník (4): vshtabletest: Fix potential memleak vsh-table: Hide vshTableRow typedef vsh-table.h: Modernize declarations vsh-table: Ensure NULL terminated arguments to vshTable*() tests/vshtabletest.c | 6 ++++-- tools/vsh-table.c | 1 + tools/vsh-table.h | 22 ++++++++++++++++------ 3 files changed, 21 insertions(+), 8 deletions(-) -- 2.32.0
In testVshTableNew() we test whether vshTableNew(NULL) allocates a table. This is expected to fail (and return NULL), because passing nothing but NULL to vshTableNew() is viewed as error. Nevertheless, if vshTableNew() did not fail and returned an allocated table it would be leaked. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/vshtabletest.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/vshtabletest.c b/tests/vshtabletest.c index 41ceec0a51..2b997753ff 100644 --- a/tests/vshtabletest.c +++ b/tests/vshtabletest.c @@ -33,7 +33,9 @@ static int testVshTableNew(const void *opaque G_GNUC_UNUSED) { - if (vshTableNew(NULL)) { + g_autoptr(vshTable) table = vshTableNew(NULL); + + if (table) { fprintf(stderr, "expected failure when passing null to vshTableNew\n"); return -1; } -- 2.32.0
There's no need for any caller to know vshTableRow typedef. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh-table.c | 1 + tools/vsh-table.h | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/vsh-table.c b/tools/vsh-table.c index 7fb0f13ee6..aa2deb8c72 100644 --- a/tools/vsh-table.c +++ b/tools/vsh-table.c @@ -31,6 +31,7 @@ #define HEX_ENCODE_LENGTH 4 /* represents length of '\xNN' */ +typedef struct _vshTableRow vshTableRow; struct _vshTableRow { char **cells; size_t ncells; diff --git a/tools/vsh-table.h b/tools/vsh-table.h index 5ce416cfa1..ad2e8ed50f 100644 --- a/tools/vsh-table.h +++ b/tools/vsh-table.h @@ -23,7 +23,6 @@ #include "vsh.h" typedef struct _vshTable vshTable; -typedef struct _vshTableRow vshTableRow; void vshTableFree(vshTable *table); vshTable *vshTableNew(const char *format, ...); -- 2.32.0
Use modern style of function declarations where the return type and function name are on two separate lines. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh-table.h | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tools/vsh-table.h b/tools/vsh-table.h index ad2e8ed50f..df647e3ba9 100644 --- a/tools/vsh-table.h +++ b/tools/vsh-table.h @@ -24,9 +24,18 @@ typedef struct _vshTable vshTable; -void vshTableFree(vshTable *table); -vshTable *vshTableNew(const char *format, ...); -int vshTableRowAppend(vshTable *table, const char *arg, ...); -void vshTablePrintToStdout(vshTable *table, vshControl *ctl); -char *vshTablePrintToString(vshTable *table, bool header); +void +vshTableFree(vshTable *table); G_DEFINE_AUTOPTR_CLEANUP_FUNC(vshTable, vshTableFree); + +vshTable * +vshTableNew(const char *format, ...); + +int +vshTableRowAppend(vshTable *table, const char *arg, ...); + +void +vshTablePrintToStdout(vshTable *table, vshControl *ctl); + +char * +vshTablePrintToString(vshTable *table, bool header); -- 2.32.0
There are two functions that take variable arguments: vshTableNew() and vshTableRowAppend(). Both expect the list of arguments to be NULL terminated. Annotate them with G_GNUC_NULL_TERMINATED to enable compile time check for this. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/vshtabletest.c | 4 ++-- tools/vsh-table.h | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/vshtabletest.c b/tests/vshtabletest.c index 2b997753ff..83d7f9b5ab 100644 --- a/tests/vshtabletest.c +++ b/tests/vshtabletest.c @@ -33,7 +33,7 @@ static int testVshTableNew(const void *opaque G_GNUC_UNUSED) { - g_autoptr(vshTable) table = vshTableNew(NULL); + g_autoptr(vshTable) table = vshTableNew(NULL, NULL); if (table) { fprintf(stderr, "expected failure when passing null to vshTableNew\n"); @@ -85,7 +85,7 @@ testVshTableRowAppend(const void *opaque G_GNUC_UNUSED) if (!table) return -1; - if (vshTableRowAppend(table, NULL) >= 0) { + if (vshTableRowAppend(table, NULL, NULL) >= 0) { fprintf(stderr, "Appending NULL shouldn't work\n"); return -1; } diff --git a/tools/vsh-table.h b/tools/vsh-table.h index df647e3ba9..18d5139aa6 100644 --- a/tools/vsh-table.h +++ b/tools/vsh-table.h @@ -29,10 +29,12 @@ vshTableFree(vshTable *table); G_DEFINE_AUTOPTR_CLEANUP_FUNC(vshTable, vshTableFree); vshTable * -vshTableNew(const char *format, ...); +vshTableNew(const char *format, ...) + G_GNUC_NULL_TERMINATED; int -vshTableRowAppend(vshTable *table, const char *arg, ...); +vshTableRowAppend(vshTable *table, const char *arg, ...) + G_GNUC_NULL_TERMINATED; void vshTablePrintToStdout(vshTable *table, vshControl *ctl); -- 2.32.0
On a Friday in 2021, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (4): vshtabletest: Fix potential memleak vsh-table: Hide vshTableRow typedef vsh-table.h: Modernize declarations vsh-table: Ensure NULL terminated arguments to vshTable*()
tests/vshtabletest.c | 6 ++++-- tools/vsh-table.c | 1 + tools/vsh-table.h | 22 ++++++++++++++++------ 3 files changed, 21 insertions(+), 8 deletions(-)
*** R-B HERE *** Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik