configure: enable -Wsuggest-attribute=format

Enable this warning, which suggests adding format attributes to some
functions, to help with format string validation.  Fix the warnings it
generates by using the new ATTR_FORMAT_PRINTF macro.

There is only one spot we can't fix, that is in modprobe.c.  The
compiler suggests we add an attribute to the kmod_set_log_fn
declaration, which we can't do, since it's not our code:

    /home/simark/src/lttng-tools/src/bin/lttng-sessiond/modprobe.c: In function ‘setup_kmod_ctx’:
    /home/simark/src/lttng-tools/src/bin/lttng-sessiond/modprobe.c:286:31: error: argument 2 of ‘kmod_set_log_fn’ might be a candidate for a format attribute [-Werror=suggest-attribute=format]
      286 |         kmod_set_log_fn(*ctx, log_kmod, NULL);
          |                               ^~~~~~~~

I don't see any other choice but to explicitly ignore that spot.

Introduce some macros to abstract how to ignore specific diagnostics.
This is useful since not all compilers support the same diagnostic
flags.  For example, telling clang to ignore -Wsuggest-attribute=format
would give an error.

Change-Id: I71278d7e2cdc66d4bbc59bd966469d0b427e963d
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/configure.ac b/configure.ac
index e9b269666f3264d2fcdb3bbdba9dff6ee36053de..083ec34f3cd3023b7e93fbf4be42705aa16f4d21 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -71,6 +71,7 @@ m4_define([WARN_FLAGS_LIST], [ dnl
   -Wmissing-parameter-type dnl
   -Wshadow dnl
   -Wno-gnu-folding-constant dnl
+  -Wsuggest-attribute=format dnl
   dnl GCC enables this with -Wall in C++, and that generates a
   dnl lot of warnings that have on average a low value to fix.
   -Wno-sign-compare dnl

diff --git a/src/bin/lttng-sessiond/modprobe.cpp b/src/bin/lttng-sessiond/modprobe.cpp
index 7e27a02eb4aba019f7108d1a8a7c0250785e4932..4c1d62b486e2b37f4181f398a7c03da58289b65d 100644 (file)
--- a/src/bin/lttng-sessiond/modprobe.cpp
+++ b/src/bin/lttng-sessiond/modprobe.cpp
@@ -248,7 +248,8 @@ static int probes_capacity;
 /**
  * @brief Logging function for libkmod integration.
  */
-static void log_kmod(void *data, int priority, const char *file, int line,
+static ATTR_FORMAT_PRINTF(6, 0)
+void log_kmod(void *data, int priority, const char *file, int line,
                const char *fn, const char *format, va_list args)
 {
        char *str;
@@ -281,7 +282,15 @@ static int setup_kmod_ctx(struct kmod_ctx **ctx)
                goto error;
        }
 
+       /*
+        * Parameter 2 of kmod_set_log_fn generates a
+        * -Wsuggest-attribute=formatkmod_set_log_fn warning that we can't fix,
+        * ignore it.
+        */
+       DIAGNOSTIC_PUSH
+       DIAGNOSTIC_IGNORE_SUGGEST_ATTRIBUTE_FORMAT
        kmod_set_log_fn(*ctx, log_kmod, NULL);
+       DIAGNOSTIC_POP
        ret = kmod_load_resources(*ctx);
        if (ret < 0) {
                ERR("Failed to load kmod library resources");

diff --git a/src/bin/lttng-sessiond/ust-metadata.cpp b/src/bin/lttng-sessiond/ust-metadata.cpp
index abe473f27ce78c6032f4de2b5d1bc869aaa31a0b..6028527c5ed27cbdb4cd7a0c885b4944b211fb0d 100644 (file)
--- a/src/bin/lttng-sessiond/ust-metadata.cpp
+++ b/src/bin/lttng-sessiond/ust-metadata.cpp
@@ -106,7 +106,7 @@ int metadata_file_append(struct ust_registry_session *session,
  * remaining space left in packet and write, since mutual exclusion
  * protects us from concurrent writes.
  */
-static
+static ATTR_FORMAT_PRINTF(2, 3)
 int lttng_metadata_printf(struct ust_registry_session *session,
                const char *fmt, ...)
 {
@@ -315,10 +315,10 @@ int ust_metadata_enum_statedump(struct ust_registry_session *session,
 
                        if (entry->start.signedness) {
                                ret = lttng_metadata_printf(session,
-                                       "%lld", (long long) entry->start.value);
+                                       "%" PRId64, entry->start.value);
                        } else {
                                ret = lttng_metadata_printf(session,
-                                       "%llu", entry->start.value);
+                                       "%" PRIu64, entry->start.value);
                        }
                        if (ret) {
                                goto end;
@@ -331,11 +331,11 @@ int ust_metadata_enum_statedump(struct ust_registry_session *session,
                        } else {
                                if (entry->end.signedness) {
                                        ret = lttng_metadata_printf(session,
-                                               " ... %lld,\n",
-                                               (long long) entry->end.value);
+                                               " ... %" PRId64 ",\n",
+                                               entry->end.value);
                                } else {
                                        ret = lttng_metadata_printf(session,
-                                               " ... %llu,\n",
+                                               " ... %" PRIu64 ",\n",
                                                entry->end.value);
                                }
                        }

diff --git a/src/common/config/session-config.cpp b/src/common/config/session-config.cpp
index 19d587c610de6aa6acd7338861b7445682d48405..c1256985ad737f3374ccf6aefd43dd7cd4f9496f 100644 (file)
--- a/src/common/config/session-config.cpp
+++ b/src/common/config/session-config.cpp
@@ -528,7 +528,7 @@ end:
        return ret >= 0 ? 0 : ret;
 }
 
-static
+static ATTR_FORMAT_PRINTF(2, 3)
 void xml_error_handler(void *ctx, const char *format, ...)
 {
        char *errMsg;

diff --git a/src/common/macros.h b/src/common/macros.h
index 74f9096c7cf3b83fded3864bc76d0b0d1e0c5fb0..99d4fb31f600fc2776348623e1821b0f76fba1b9 100644 (file)
--- a/src/common/macros.h
+++ b/src/common/macros.h
@@ -75,6 +75,24 @@ void *zmalloc(size_t len)
 
 #define ASSERT_LOCKED(lock) LTTNG_ASSERT(pthread_mutex_trylock(&lock))
 
+/* Attribute suitable to tag functions as having printf()-like arguments. */
+#define ATTR_FORMAT_PRINTF(_string_index, _first_to_check) \
+       __attribute__((format(printf, _string_index, _first_to_check)))
+
+/* Macros used to ignore specific compiler diagnostics. */
+
+#define DIAGNOSTIC_PUSH _Pragma("GCC diagnostic push")
+#define DIAGNOSTIC_POP _Pragma("GCC diagnostic pop")
+
+#if defined(__clang__)
+  /* Clang */
+# define DIAGNOSTIC_IGNORE_SUGGEST_ATTRIBUTE_FORMAT
+#else
+  /* GCC */
+# define DIAGNOSTIC_IGNORE_SUGGEST_ATTRIBUTE_FORMAT \
+       _Pragma("GCC diagnostic ignored \"-Wsuggest-attribute=format\"")
+#endif
+
 /*
  * lttng_strncpy returns 0 on success, or nonzero on failure.
  * It checks that the @src string fits into @dst_len before performing

diff --git a/tests/utils/testapp/gen-ns-events/gen-ns-events.cpp b/tests/utils/testapp/gen-ns-events/gen-ns-events.cpp
index 7c2b6478c4520f526a9dbca17c7d4d12d1bb650f..78b962d19bb71a0e214fcd6a0f4ec41511eb73f1 100644 (file)
--- a/tests/utils/testapp/gen-ns-events/gen-ns-events.cpp
+++ b/tests/utils/testapp/gen-ns-events/gen-ns-events.cpp
@@ -20,6 +20,7 @@
 #include <unistd.h>
 
 #include <common/compat/tid.h>
+#include <common/macros.h>
 
 #include "signal-helper.h"
 #include "utils.h"
@@ -74,7 +75,8 @@ static struct poptOption opts[] = {
        { NULL, 0, 0, NULL, 0 }
 };
 
-static void debug_printf(const char *format, ...)
+static ATTR_FORMAT_PRINTF(1, 2)
+void debug_printf(const char *format, ...)
 {
        va_list args;
        va_start(args, format);

diff --git a/tests/utils/testapp/gen-ust-events-ns/gen-ust-events-ns.cpp b/tests/utils/testapp/gen-ust-events-ns/gen-ust-events-ns.cpp
index dadfc3d3a00301bc81ba044ef19403585812ccda..c916a961611c3d1f7aeae903c7f46190432f552c 100644 (file)
--- a/tests/utils/testapp/gen-ust-events-ns/gen-ust-events-ns.cpp
+++ b/tests/utils/testapp/gen-ust-events-ns/gen-ust-events-ns.cpp
@@ -19,6 +19,7 @@
 #include <unistd.h>
 
 #include <common/compat/tid.h>
+#include <common/macros.h>
 
 #include "signal-helper.h"
 #include "utils.h"
@@ -77,7 +78,8 @@ struct poptOption opts[] = {
        { NULL, 0, 0, NULL, 0 }
 };
 
-static void debug_printf(const char *format, ...)
+static ATTR_FORMAT_PRINTF(1, 2)
+void debug_printf(const char *format, ...)
 {
        va_list args;
        va_start(args, format);

diff --git a/tests/utils/xml-utils/validate_xml.cpp b/tests/utils/xml-utils/validate_xml.cpp
index bb67e56e17edea75706b56c50ec072c49f2433be..56002ed7cd4769e7b1a3c83ed2fa106e2723352b 100644 (file)
--- a/tests/utils/xml-utils/validate_xml.cpp
+++ b/tests/utils/xml-utils/validate_xml.cpp
@@ -27,6 +27,8 @@
 #include <lttng/lttng-error.h>
 #include <common/macros.h>
 
+#include <common/macros.h>
+
 struct validation_ctx {
        xmlSchemaParserCtxtPtr parser_ctx;
        xmlSchemaPtr schema;
@@ -38,7 +40,7 @@ enum command_err_code {
        CMD_ERROR
 };
 
-static
+static ATTR_FORMAT_PRINTF(2, 3)
 void xml_error_handler(void *ctx, const char *format, ...)
 {
        char *err_msg;