[PATCH 0/2] lib: Annotate more functions as NULL terminated

I'm working on something that's calling qemuMonitorCreateObjectProps() and was getting random errors only to find out I was missing NULL sentinel. This sparked me to look at other functions that might be missing the G_GNUC_NULL_TERMINATED attribute too and found some. Michal Prívozník (2): lib: Annotate more function as NULL terminated qemumonitortestutils: Fix G_GNUC_PRINTF annotation of qemuMonitorTestAddErrorResponse() src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 2 +- tests/qemumigrationcookiexmltest.c | 2 +- tests/qemumonitortestutils.c | 2 +- tests/qemumonitortestutils.h | 3 ++- tests/testutils.h | 3 ++- tests/testutilsqemuschema.h | 3 ++- tools/vsh.c | 2 +- 8 files changed, 12 insertions(+), 8 deletions(-) -- 2.44.1

While __attribute((sentinel)) (exposed by glib under G_GNUC_NULL_TERMINATED macro) is a gcc extension, it's supported by clang too. It's already being used throughout our code but some functions that take variadic arguments and expect NULL at the end were lacking such annotation. Fill them in. After this, there are still some functions left untouched because they expect a different sentinel than NULL. Unfortunately, glib does not provide macro for different sentinels. We may come up with our own, but let's save that for future work. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 2 +- tests/qemumigrationcookiexmltest.c | 2 +- tests/testutils.h | 3 ++- tests/testutilsqemuschema.h | 3 ++- tools/vsh.c | 2 +- 6 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6e81945201..b78f539c85 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -947,7 +947,8 @@ int qemuMonitorDelDevice(qemuMonitor *mon, int qemuMonitorCreateObjectProps(virJSONValue **propsret, const char *type, const char *alias, - ...); + ...) + G_GNUC_NULL_TERMINATED; int qemuMonitorAddObject(qemuMonitor *mon, virJSONValue **props, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index eb84a3d938..c5e758e7f8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -468,7 +468,7 @@ qemuMonitorJSONHasError(virJSONValue *reply, * * Returns 0 on success and -1 on error. */ -static int +static int G_GNUC_NULL_TERMINATED qemuMonitorJSONTransactionAdd(virJSONValue *actions, const char *cmdname, ...) diff --git a/tests/qemumigrationcookiexmltest.c b/tests/qemumigrationcookiexmltest.c index 5270e3a7e7..bc0f68b8c5 100644 --- a/tests/qemumigrationcookiexmltest.c +++ b/tests/qemumigrationcookiexmltest.c @@ -39,7 +39,7 @@ static virQEMUDriver driver; static virBuffer testnamebuf = VIR_BUFFER_INITIALIZER; -static const char * +static const char * G_GNUC_NULL_TERMINATED tn(const char *str, ...) { va_list ap; diff --git a/tests/testutils.h b/tests/testutils.h index e5469c5aa0..e22324e06d 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -113,7 +113,8 @@ void virTestFakeRootDirCleanup(char *fakerootdir); int virTestMain(int argc, char **argv, int (*func)(void), - ...); + ...) + G_GNUC_NULL_TERMINATED; /* Setup, then call func() */ #define VIR_TEST_MAIN(func) \ diff --git a/tests/testutilsqemuschema.h b/tests/testutilsqemuschema.h index cb1e6da69e..191e763936 100644 --- a/tests/testutilsqemuschema.h +++ b/tests/testutilsqemuschema.h @@ -40,7 +40,8 @@ testQEMUSchemaValidateCommand(const char *command, int testQEMUSchemaEntryMatchTemplate(virJSONValue *schemaentry, - ...); + ...) + G_GNUC_NULL_TERMINATED; virJSONValue * diff --git a/tools/vsh.c b/tools/vsh.c index 6cc1f60d87..9fbb1f9349 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2418,7 +2418,7 @@ vshCloseLogFile(vshControl *ctl) } #ifndef WIN32 -static void +static void G_GNUC_NULL_TERMINATED vshPrintRaw(vshControl *ctl, ...) { va_list ap; -- 2.44.1

The qemuMonitorTestAddErrorResponse() function is a printf-like function. But the annotation was mistakenly done in .c file instead of corresponding .h file rendering the annotation ineffective. Move the annotation to the header file. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemumonitortestutils.c | 2 +- tests/qemumonitortestutils.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 4e6a9371cb..88a369188e 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -172,7 +172,7 @@ qemuMonitorTestAddInvalidCommandResponse(qemuMonitorTest *test, } -int G_GNUC_PRINTF(2, 3) +int qemuMonitorTestAddErrorResponse(qemuMonitorTest *test, const char *errmsg, ...) { va_list msgargs; diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h index edd38d8df6..6d26526f60 100644 --- a/tests/qemumonitortestutils.h +++ b/tests/qemumonitortestutils.h @@ -52,7 +52,8 @@ qemuMonitorTestItemGetPrivateData(qemuMonitorTestItem *item); int qemuMonitorTestAddErrorResponse(qemuMonitorTest *test, const char *errmsg, - ...); + ...) + G_GNUC_PRINTF(2, 3); void qemuMonitorTestAllowUnusedCommands(qemuMonitorTest *test); -- 2.44.1

On a Wednesday in 2024, Michal Privoznik wrote:
I'm working on something that's calling qemuMonitorCreateObjectProps() and was getting random errors only to find out I was missing NULL sentinel. This sparked me to look at other functions that might be missing the G_GNUC_NULL_TERMINATED attribute too and found some.
Michal Prívozník (2): lib: Annotate more function as NULL terminated qemumonitortestutils: Fix G_GNUC_PRINTF annotation of qemuMonitorTestAddErrorResponse()
src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 2 +- tests/qemumigrationcookiexmltest.c | 2 +- tests/qemumonitortestutils.c | 2 +- tests/qemumonitortestutils.h | 3 ++- tests/testutils.h | 3 ++- tests/testutilsqemuschema.h | 3 ++- tools/vsh.c | 2 +- 8 files changed, 12 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Michal Privoznik