configure: enable -Wsuggest-attribute=format

The -Wsuggest-attribute=format warning makes the compiler suggest places
where __attribute__((format(...))) would likely be useful.  This patch
turns it on and adds such attributes everywhere my compiler (GCC 9)
suggested to add them.

In cases where we re-use the same format string multiple times, like:

    const char *const msg = "Hello %d\n";

    BT_LOGE(msg, some_value);
    BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_MESSAGE_ITERATOR("modname", msg, some_value);

... my compiler (gcc (Arch Linux 9.2.1+20200130-2) 9.2.1 20200130)
complains that the format string is not a literal, even though the `msg`
variable is const and assigned a literal.  I've replaced these with a
macro.

Change-Id: I40dd2e70649ec2b651e0109097c217ca9557ad69
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/3232
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/configure.ac b/configure.ac
index 7ebcf2ad40c553d1e90e674be83dd11375c892d1..ea652c734c737d28259147fb20f4d3af4103e7a9 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -676,6 +676,7 @@ AX_APPEND_COMPILE_FLAGS([ dnl
                -Wredundant-decls dnl
                -Wshadow dnl
                -Wjump-misses-init dnl
+               -Wsuggest-attribute=format dnl
                -Wtautological-constant-out-of-range-compare dnl
                -Wnested-externs dnl
                -Wwrite-strings dnl

diff --git a/include/babeltrace2/babeltrace.h b/include/babeltrace2/babeltrace.h
index 3f919cf30a6ea65f29caf4b5de8fa6ac30d485e1..c94576a7810f825a2cfec808eba94a4f5dc1b191 100644 (file)
--- a/include/babeltrace2/babeltrace.h
+++ b/include/babeltrace2/babeltrace.h
@@ -44,6 +44,23 @@
 # define __BT_UPCAST_CONST(_type, _p)  ((const _type *) (_p))
 #endif
 
+/*
+ * Internal: attribute suitable to tag functions as having printf()-like
+ * arguments.
+ *
+ * We always define `__USE_MINGW_ANSI_STDIO` when building with MinGW, so use
+ * `gnu_printf` directly rather than `__MINGW_PRINTF_FORMAT` (which would require
+ * including `stdio.h`).
+ */
+#ifdef __MINGW32__
+# define __BT_PRINTF_FORMAT gnu_printf
+#else
+# define __BT_PRINTF_FORMAT printf
+#endif
+
+#define __BT_ATTR_FORMAT_PRINTF(_string_index, _first_to_check) \
+               __attribute__((format(__BT_PRINTF_FORMAT, _string_index, _first_to_check)))
+
 #include <babeltrace2/error-reporting.h>
 #include <babeltrace2/graph/component-class-dev.h>
 #include <babeltrace2/graph/component-class.h>

diff --git a/include/babeltrace2/error-reporting.h b/include/babeltrace2/error-reporting.h
index 8539ddf803daf6f5adb66ac553ea5c10a2fdf427..4916ef671f63d7cc36530d159f505977416f3028 100644 (file)
--- a/include/babeltrace2/error-reporting.h
+++ b/include/babeltrace2/error-reporting.h
@@ -629,7 +629,7 @@ if no \bt_plugin provides the class of \bt_p{self_component}, with:
     Calls this function with \c __FILE__ and \c __LINE__ as the
     \bt_p{file_name} and \bt_p{line_number} parameters.
 */
-extern
+extern __BT_ATTR_FORMAT_PRINTF(4, 5)
 bt_current_thread_error_append_cause_status
 bt_current_thread_error_append_cause_from_component(
                bt_self_component *self_component, const char *file_name,
@@ -728,7 +728,7 @@ if no \bt_plugin provides the component class of
     Calls this function with \c __FILE__ and \c __LINE__ as the
     \bt_p{file_name} and \bt_p{line_number} parameters.
 */
-extern
+extern __BT_ATTR_FORMAT_PRINTF(4, 5)
 bt_current_thread_error_append_cause_status
 bt_current_thread_error_append_cause_from_message_iterator(
                bt_self_message_iterator *self_message_iterator,
@@ -820,7 +820,7 @@ if no \bt_plugin provides \bt_p{self_component_class}, with:
     Calls this function with \c __FILE__ and \c __LINE__ as the
     \bt_p{file_name} and \bt_p{line_number} parameters.
 */
-extern
+extern __BT_ATTR_FORMAT_PRINTF(4, 5)
 bt_current_thread_error_append_cause_status
 bt_current_thread_error_append_cause_from_component_class(
                bt_self_component_class *self_component_class,
@@ -884,7 +884,7 @@ format string parameter \bt_p{message_format}.
     Calls this function with \c __FILE__ and \c __LINE__ as the
     \bt_p{file_name} and \bt_p{line_number} parameters.
 */
-extern
+extern __BT_ATTR_FORMAT_PRINTF(4, 5)
 bt_current_thread_error_append_cause_status
 bt_current_thread_error_append_cause_from_unknown(
                const char *module_name, const char *file_name,

diff --git a/src/argpar/argpar.c b/src/argpar/argpar.c
index ad56838fad853f15629a06f25dcec82b91d07d85..5bd2652e00bd195f386c75c10bf339f996a52325 100644 (file)
--- a/src/argpar/argpar.c
+++ b/src/argpar/argpar.c
@@ -35,7 +35,13 @@
 
 #define ARGPAR_ASSERT(_cond) assert(_cond)
 
-static
+#ifdef __MINGW_PRINTF_FORMAT
+# define ARGPAR_PRINTF_FORMAT __MINGW_PRINTF_FORMAT
+#else
+# define ARGPAR_PRINTF_FORMAT printf
+#endif
+
+static __attribute__((format(ARGPAR_PRINTF_FORMAT, 1, 0)))
 char *argpar_vasprintf(const char *fmt, va_list args)
 {
        int len1, len2;
@@ -65,7 +71,7 @@ end:
 }
 
 
-static
+static __attribute__((format(ARGPAR_PRINTF_FORMAT, 1, 2)))
 char *argpar_asprintf(const char *fmt, ...)
 {
        va_list args;
@@ -78,7 +84,7 @@ char *argpar_asprintf(const char *fmt, ...)
        return str;
 }
 
-static
+static __attribute__((format(ARGPAR_PRINTF_FORMAT, 2, 3)))
 bool argpar_string_append_printf(char **str, const char *fmt, ...)
 {
        char *new_str = NULL;

diff --git a/src/bindings/python/bt2/bt2/native_bt.i b/src/bindings/python/bt2/bt2/native_bt.i
index 2300cee42f65c856576a88166f65fb8871a6b5c2..700c1d1712a1b9ce4ee4d5d0cf277b50148ba5a4 100644 (file)
--- a/src/bindings/python/bt2/bt2/native_bt.i
+++ b/src/bindings/python/bt2/bt2/native_bt.i
@@ -207,6 +207,12 @@ void bt_bt2_exit_handler(void);
  */
 #define __BT_IN_BABELTRACE_H
 
+/*
+ * Define `__BT_ATTR_FORMAT_PRINTF` to nothing, otherwise SWIG fails to parse
+ * the included header files that use it.
+ */
+#define __BT_ATTR_FORMAT_PRINTF(_string_index, _first_to_check)
+
 /* Common types */
 %include <babeltrace2/types.h>
 

diff --git a/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h b/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h
index 612c9328fd8c6a60a05a528e08a735d2318e0600..cb1bd84a4170443fdc8d386ef1a496a104410220 100644 (file)
--- a/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h
+++ b/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h
@@ -70,9 +70,10 @@ bt_value *bt_bt2_auto_discover_source_components(const bt_value *inputs,
 
        result = bt_value_map_create();
        if (!result) {
-               static const char * const err = "Failed to create a map value.";
-               BT_LOGE_STR(err);
-               BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_UNKNOWN(module_name, err);
+#define BT_FMT "Failed to create a map value."
+               BT_LOGE_STR(BT_FMT);
+               BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_UNKNOWN(module_name, BT_FMT);
+#undef BT_FMT
                PyErr_NoMemory();
                goto end;
        }

diff --git a/src/bindings/python/bt2/bt2/native_bt_component_class.i.h b/src/bindings/python/bt2/bt2/native_bt_component_class.i.h
index f0ba04258d2a331705d3586a67b5c247162e2b7b..a507c280096dd39783a7a8010ccd719eea4aa204 100644 (file)
--- a/src/bindings/python/bt2/bt2/native_bt_component_class.i.h
+++ b/src/bindings/python/bt2/bt2/native_bt_component_class.i.h
@@ -43,6 +43,8 @@
  * a BT component object instance.
  */
 
+#define BT_FMT_SWIG_ALLOC_FAILED "Failed to create a SWIG pointer object."
+
 static GHashTable *bt_cc_ptr_to_py_cls;
 
 static
@@ -234,7 +236,7 @@ bt_component_class_initialize_method_status component_class_init(
                SWIGTYPE_p_bt_value, 0);
        if (!py_params_ptr) {
                BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
-                       "Failed to create a SWIG pointer object.");
+                       BT_FMT_SWIG_ALLOC_FAILED);
                goto error;
        }
 
@@ -242,7 +244,7 @@ bt_component_class_initialize_method_status component_class_init(
                self_comp_cls_type_swig_type, 0);
        if (!py_comp_ptr) {
                BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
-                       "Failed to create a SWIG pointer object.");
+                       BT_FMT_SWIG_ALLOC_FAILED);
                goto error;
        }
 
@@ -324,7 +326,7 @@ component_class_get_supported_mip_versions(
                SWIGTYPE_p_bt_value, 0);
        if (!py_params_ptr) {
                BT_LOG_WRITE_CUR_LVL(BT_LOG_ERROR, log_level, BT_LOG_TAG,
-                       "Failed to create a SWIG pointer object.");
+                       BT_FMT_SWIG_ALLOC_FAILED);
                goto error;
        }
 
@@ -790,7 +792,7 @@ bt_component_class_port_connected_method_status component_class_port_connected(
                self_component_port_swig_type, 0);
        if (!py_self_port_ptr) {
                BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
-                       "Failed to create a SWIG pointer object.");
+                       BT_FMT_SWIG_ALLOC_FAILED);
                status = __BT_FUNC_STATUS_MEMORY_ERROR;
                goto end;
        }
@@ -799,7 +801,7 @@ bt_component_class_port_connected_method_status component_class_port_connected(
                other_port_swig_type, 0);
        if (!py_other_port_ptr) {
                BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
-                       "Failed to create a SWIG pointer object.");
+                       BT_FMT_SWIG_ALLOC_FAILED);
                status = __BT_FUNC_STATUS_MEMORY_ERROR;
                goto end;
        }
@@ -965,7 +967,7 @@ bt_component_class_query_method_status component_class_query(
                SWIGTYPE_p_bt_value, 0);
        if (!py_params_ptr) {
                BT_LOG_WRITE_CUR_LVL(BT_LOG_ERROR, log_level, BT_LOG_TAG,
-                       "Failed to create a SWIG pointer object.");
+                       BT_FMT_SWIG_ALLOC_FAILED);
                goto error;
        }
 
@@ -974,7 +976,7 @@ bt_component_class_query_method_status component_class_query(
                SWIGTYPE_p_bt_private_query_executor, 0);
        if (!py_priv_query_exec_ptr) {
                BT_LOG_WRITE_CUR_LVL(BT_LOG_ERROR, log_level, BT_LOG_TAG,
-                       "Failed to create a SWIG pointer object.");
+                       BT_FMT_SWIG_ALLOC_FAILED);
                goto error;
        }
 
@@ -1120,12 +1122,10 @@ component_class_message_iterator_init(
        py_iter_ptr = SWIG_NewPointerObj(SWIG_as_voidptr(self_message_iterator),
                SWIGTYPE_p_bt_self_message_iterator, 0);
        if (!py_iter_ptr) {
-               const char *err = "Failed to create a SWIG pointer object.";
-
                BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
-                       "%s", err);
+                       BT_FMT_SWIG_ALLOC_FAILED);
                BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_MESSAGE_ITERATOR(
-                       self_message_iterator, err);
+                       self_message_iterator, BT_FMT_SWIG_ALLOC_FAILED);
                goto error;
        }
 
@@ -1159,12 +1159,10 @@ component_class_message_iterator_init(
        py_config_ptr = SWIG_NewPointerObj(SWIG_as_voidptr(config),
                SWIGTYPE_p_bt_self_message_iterator_configuration, 0);
        if (!py_config_ptr) {
-               const char *err = "Failed to create a SWIG pointer object";
-
                BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
-                       "%s", err);
+                       BT_FMT_SWIG_ALLOC_FAILED);
                BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_MESSAGE_ITERATOR(
-                       self_message_iterator, err);
+                       self_message_iterator, BT_FMT_SWIG_ALLOC_FAILED);
                goto error;
        }
 
@@ -1172,12 +1170,10 @@ component_class_message_iterator_init(
                SWIG_as_voidptr(self_component_port_output),
                SWIGTYPE_p_bt_self_component_port_output, 0);
        if (!py_component_port_output_ptr) {
-               const char *err = "Failed to create a SWIG pointer object.";
-
                BT_COMP_LOG_CUR_LVL(BT_LOG_ERROR, log_level, self_component,
-                       "%s", err);
+                       "%s", BT_FMT_SWIG_ALLOC_FAILED);
                BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_MESSAGE_ITERATOR(
-                       self_message_iterator, err);
+                       self_message_iterator, BT_FMT_SWIG_ALLOC_FAILED);
                goto error;
        }
 

diff --git a/src/common/common.h b/src/common/common.h
index b5990a885395036a853288d3b7fe330f203e94bb..1abe37ec11b094bf80713ad212a462f6e6f32f93 100644 (file)
--- a/src/common/common.h
+++ b/src/common/common.h
@@ -823,7 +823,7 @@ end:
  * bt_g_string_append_printf cannot be inlined because it expects a
  * variadic argument list.
  */
-BT_HIDDEN
+BT_HIDDEN __BT_ATTR_FORMAT_PRINTF(2, 3)
 int bt_common_g_string_append_printf(GString *str, const char *fmt, ...);
 
 static inline

diff --git a/src/lib/error.h b/src/lib/error.h
index 78f9c532af4dfa0900d51adee9cb0a5cdc42a0e5..63960e2e913172ef687d8d9f413fe84beb991e09 100644 (file)
--- a/src/lib/error.h
+++ b/src/lib/error.h
@@ -93,25 +93,25 @@ struct bt_error *bt_error_create(void);
 BT_HIDDEN
 void bt_error_destroy(struct bt_error *error);
 
-BT_HIDDEN
+BT_HIDDEN __BT_ATTR_FORMAT_PRINTF(5, 0)
 int bt_error_append_cause_from_unknown(struct bt_error *error,
                const char *module_name, const char *file_name,
                uint64_t line_no, const char *msg_fmt, va_list args);
 
-BT_HIDDEN
+BT_HIDDEN __BT_ATTR_FORMAT_PRINTF(5, 0)
 int bt_error_append_cause_from_component(
                struct bt_error *error, bt_self_component *self_comp,
                const char *file_name, uint64_t line_no,
                const char *msg_fmt, va_list args);
 
-BT_HIDDEN
+BT_HIDDEN __BT_ATTR_FORMAT_PRINTF(5, 0)
 int bt_error_append_cause_from_component_class(
                struct bt_error *error,
                bt_self_component_class *self_comp_class,
                const char *file_name, uint64_t line_no,
                const char *msg_fmt, va_list args);
 
-BT_HIDDEN
+BT_HIDDEN __BT_ATTR_FORMAT_PRINTF(5, 0)
 int bt_error_append_cause_from_message_iterator(
                struct bt_error *error, bt_self_message_iterator *self_iter,
                const char *file_name, uint64_t line_no,

diff --git a/src/logging/log.c b/src/logging/log.c
index 349204e8148fa24f047dc6fd0b98ff39f6017d57..f13694ea7233353361e485625222844ce1bb417c 100644 (file)
--- a/src/logging/log.c
+++ b/src/logging/log.c
@@ -1193,7 +1193,8 @@ static void put_src(bt_log_message *const msg, const src_location *const src)
 #endif
 }
 
-static void put_msg(bt_log_message *const msg,
+static _BT_LOG_PRINTFLIKE(2, 0)
+void put_msg(bt_log_message *const msg,
                                        const char *const fmt, va_list va)
 {
        int n;
@@ -1268,7 +1269,8 @@ void bt_log_set_output_v(const unsigned mask, void *const arg,
        _bt_log_global_output.callback = callback;
 }
 
-static void _bt_log_write_imp(
+static _BT_LOG_PRINTFLIKE(6, 0)
+void _bt_log_write_imp(
                const bt_log_spec *log,
                const src_location *const src, const mem_block *const mem,
                const int lvl, const char *const tag, const char *const fmt, va_list va)

diff --git a/src/plugins/common/param-validation/param-validation.h b/src/plugins/common/param-validation/param-validation.h
index 27277edfa6ceff1d81f3948536daa46a6803e75d..9acfb6fd0f2727b13d3da775d7644a413c01bac5 100644 (file)
--- a/src/plugins/common/param-validation/param-validation.h
+++ b/src/plugins/common/param-validation/param-validation.h
@@ -31,12 +31,6 @@
 
 #include <common/macros.h>
 
-#ifdef __MINGW_PRINTF_FORMAT
-# define BT_PRINTF_FORMAT __MINGW_PRINTF_FORMAT
-#else
-# define BT_PRINTF_FORMAT printf
-#endif
-
 struct bt_param_validation_context;
 struct bt_param_validation_value_descr;
 
@@ -106,8 +100,7 @@ enum bt_param_validation_status bt_param_validation_validate(
                const struct bt_param_validation_map_value_entry_descr *entries,
                gchar **error);
 
-BT_HIDDEN
-__attribute__((format(BT_PRINTF_FORMAT, 2, 3)))
+BT_HIDDEN __BT_ATTR_FORMAT_PRINTF(2, 3)
 enum bt_param_validation_status bt_param_validation_error(
                struct bt_param_validation_context *ctx,
                const char *format, ...);