From d22ad5f818289bb10faa814c2ecef071ec0c2c67 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 20 Aug 2021 15:19:42 -0400 Subject: [PATCH] configure: enable -Wsuggest-attribute=format MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Jérémie Galarneau --- configure.ac | 1 + src/bin/lttng-sessiond/modprobe.cpp | 11 ++++++++++- src/bin/lttng-sessiond/ust-metadata.cpp | 12 ++++++------ src/common/config/session-config.cpp | 2 +- src/common/macros.h | 18 ++++++++++++++++++ .../testapp/gen-ns-events/gen-ns-events.cpp | 4 +++- .../gen-ust-events-ns/gen-ust-events-ns.cpp | 4 +++- tests/utils/xml-utils/validate_xml.cpp | 4 +++- 8 files changed, 45 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index e9b269666..083ec34f3 100644 --- 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 7e27a02eb..4c1d62b48 100644 --- 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 abe473f27..6028527c5 100644 --- 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 19d587c61..c1256985a 100644 --- 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 74f9096c7..99d4fb31f 100644 --- 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 7c2b6478c..78b962d19 100644 --- 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 #include +#include #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 dadfc3d3a..c916a9616 100644 --- 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 #include +#include #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 bb67e56e1..56002ed7c 100644 --- a/tests/utils/xml-utils/validate_xml.cpp +++ b/tests/utils/xml-utils/validate_xml.cpp @@ -27,6 +27,8 @@ #include #include +#include + 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; -- 2.34.1