From 8b305066676fc7aa433e8eb668f9de8802008025 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 16 Mar 2020 18:38:45 -0400 Subject: [PATCH] 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 Reviewed-on: https://review.lttng.org/c/babeltrace/+/3232 Tested-by: jenkins Reviewed-by: Philippe Proulx --- configure.ac | 1 + include/babeltrace2/babeltrace.h | 17 ++++++++++ include/babeltrace2/error-reporting.h | 8 ++--- src/argpar/argpar.c | 12 +++++-- src/bindings/python/bt2/bt2/native_bt.i | 6 ++++ .../python/bt2/bt2/native_bt_autodisc.i.h | 7 ++-- .../bt2/bt2/native_bt_component_class.i.h | 34 ++++++++----------- src/common/common.h | 2 +- src/lib/error.h | 8 ++--- src/logging/log.c | 6 ++-- .../param-validation/param-validation.h | 9 +---- 11 files changed, 66 insertions(+), 44 deletions(-) diff --git a/configure.ac b/configure.ac index 7ebcf2ad..ea652c73 100644 --- 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 3f919cf3..c94576a7 100644 --- 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 #include #include diff --git a/include/babeltrace2/error-reporting.h b/include/babeltrace2/error-reporting.h index 8539ddf8..4916ef67 100644 --- 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 ad56838f..5bd2652e 100644 --- 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 2300cee4..700c1d17 100644 --- 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 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 612c9328..cb1bd84a 100644 --- 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 f0ba0425..a507c280 100644 --- 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 b5990a88..1abe37ec 100644 --- 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 78f9c532..63960e2e 100644 --- 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 349204e8..f13694ea 100644 --- 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 27277edf..9acfb6fd 100644 --- a/src/plugins/common/param-validation/param-validation.h +++ b/src/plugins/common/param-validation/param-validation.h @@ -31,12 +31,6 @@ #include -#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, ...); -- 2.34.1