From 900eef7317d3e8b350a93fb8782639845c62c2bd Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 15 Mar 2020 16:12:52 -0400 Subject: [PATCH] configure: remove -Wno-format-nonliteral MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This patch removes all the offending instances of -Wformat-nonliteral and removes -Wno-format-nonliteral from the warning flags we enable. In cases where we re-use the same format string multiple times, like: const char *const msg = "Hello %d\n"; if (something) { pass(msg, value); } else { fail(msg, 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. In other places, we manually manipulate and craft format strings, to I've just disabled the warning there. The generated SWIG wrapper for the error appending functions cause some warnings like this: bt2/native_bt.c: In function ‘_wrap_current_thread_error_append_cause_from_component__varargs__’: bt2/native_bt.c:7839:3: error: format not a string literal, argument types not checked [-Werror=format-nonliteral] 7839 | result = (bt_current_thread_error_append_cause_status)bt_current_thread_error_append_cause_from_component(arg1,(char const *)arg2,arg3,(char const *)arg4,ar g5); | ^~~~~~ Since we don't actually need these functions from the Python bindings, I've made SWIG not generate a wrapper for them. Change-Id: Ic6834dc44abce7be8e113e9cbf3f33a3f09809e2 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/3233 Reviewed-by: Philippe Proulx Tested-by: jenkins --- configure.ac | 1 - src/autodisc/autodisc.c | 7 ++++--- src/bindings/python/bt2/bt2/native_bt.i | 9 +++++++++ .../python/bt2/bt2/native_bt_component_class.i.h | 8 ++++---- src/common/common.c | 5 ++++- src/lib/lib-logging.c | 3 +++ src/plugins/text/details/write.c | 6 ++++++ src/string-format/format-error.c | 3 +++ tests/param-validation/test_param_validation.c | 9 ++++----- tests/utils/tap/tap.c | 2 +- 10 files changed, 38 insertions(+), 15 deletions(-) diff --git a/configure.ac b/configure.ac index 70c0a936..8ab012e0 100644 --- a/configure.ac +++ b/configure.ac @@ -665,7 +665,6 @@ AX_APPEND_COMPILE_FLAGS([ dnl -Wnested-externs dnl -Wwrite-strings dnl -Wformat=2 dnl - -Wno-format-nonliteral dnl -Wstrict-aliasing dnl -Wmissing-noreturn dnl -Winit-self dnl diff --git a/src/autodisc/autodisc.c b/src/autodisc/autodisc.c index 52437605..1a4f07dc 100644 --- a/src/autodisc/autodisc.c +++ b/src/autodisc/autodisc.c @@ -612,18 +612,19 @@ auto_source_discovery_internal_status auto_discover_source_for_input_as_dir_or_f dir = g_dir_open(input->str, 0, &error); if (!dir) { - const char *fmt = "Failed to open directory %s: %s"; - BT_LOGW(fmt, input->str, error->message); +#define BT_FMT "Failed to open directory %s: %s" + BT_LOGW(BT_FMT, input->str, error->message); if (error->code == G_FILE_ERROR_ACCES) { /* This is not a fatal error, we just skip it. */ status = AUTO_SOURCE_DISCOVERY_INTERNAL_STATUS_NO_MATCH; goto end; } else { - BT_AUTODISC_LOGE_APPEND_CAUSE(fmt, input->str, + BT_AUTODISC_LOGE_APPEND_CAUSE(BT_FMT, input->str, error->message); goto error; } +#undef BT_FMT } saved_input_len = input->len; diff --git a/src/bindings/python/bt2/bt2/native_bt.i b/src/bindings/python/bt2/bt2/native_bt.i index 6a79cfef..21a869e6 100644 --- a/src/bindings/python/bt2/bt2/native_bt.i +++ b/src/bindings/python/bt2/bt2/native_bt.i @@ -182,6 +182,15 @@ typedef uint64_t bt_listener_id; void bt_bt2_init_from_bt2(void); void bt_bt2_exit_handler(void); +/* + * These functions cause some -Wformat-nonliteral warnings, but we don't need + * them. Ignore them, so that we can keep the warning turned on. + */ +%ignore bt_current_thread_error_append_cause_from_component; +%ignore bt_current_thread_error_append_cause_from_component_class; +%ignore bt_current_thread_error_append_cause_from_message_iterator; +%ignore bt_current_thread_error_append_cause_from_unknown; + /* * Define `__BT_IN_BABELTRACE_H` to allow specific headers to be * included. This remains defined as long as we don't include the main 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 b2c61622..3759335c 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 @@ -984,12 +984,12 @@ bt_component_class_query_method_status component_class_query( status = py_exc_to_status_component_class_clear(self_component_class, log_level); if (status < 0) { - static const char *fmt = - "Failed to call Python class's _bt_query_from_native() method: py-cls-addr=%p"; +#define BT_FMT "Failed to call Python class's _bt_query_from_native() method: py-cls-addr=%p" BT_LOG_WRITE_CUR_LVL(BT_LOG_WARNING, log_level, BT_LOG_TAG, - fmt, py_cls); + BT_FMT, py_cls); BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_COMPONENT_CLASS( - self_component_class, fmt, py_cls); + self_component_class, BT_FMT, py_cls); +#undef BT_FMT } goto end; } diff --git a/src/common/common.c b/src/common/common.c index 048e3d27..85920c10 100644 --- a/src/common/common.c +++ b/src/common/common.c @@ -1454,8 +1454,11 @@ size_t bt_common_get_page_size(int log_level) size_t _tmp_fmt_size = (size_t) (fmt_ch - *out_fmt_ch); \ strncpy(_tmp_fmt, *out_fmt_ch, _tmp_fmt_size); \ _tmp_fmt[_tmp_fmt_size] = '\0'; \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wformat-nonliteral\"") \ _count = snprintf(*buf_ch, _size, _tmp_fmt, __VA_ARGS__); \ - BT_ASSERT_DBG(_count >= 0); \ + _Pragma("GCC diagnostic pop") \ + BT_ASSERT_DBG(_count >= 0); \ *buf_ch += MIN(_count, _size); \ } while (0) diff --git a/src/lib/lib-logging.c b/src/lib/lib-logging.c index 88be5c76..e4d3ac20 100644 --- a/src/lib/lib-logging.c +++ b/src/lib/lib-logging.c @@ -331,6 +331,8 @@ static inline void format_field_integer_extended(char **buf_ch, fmt = ", %svalue=%" PRIx64; } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-nonliteral" if (field_class->common.type == BT_FIELD_CLASS_TYPE_SIGNED_INTEGER || field_class->common.type == BT_FIELD_CLASS_TYPE_SIGNED_ENUMERATION) { if (!fmt) { @@ -345,6 +347,7 @@ static inline void format_field_integer_extended(char **buf_ch, BUF_APPEND(fmt, PRFIELD(integer->value.u)); } +#pragma GCC diagnostic pop } static inline void format_field(char **buf_ch, bool extended, diff --git a/src/plugins/text/details/write.c b/src/plugins/text/details/write.c index 51490a14..8409a96c 100644 --- a/src/plugins/text/details/write.c +++ b/src/plugins/text/details/write.c @@ -89,7 +89,10 @@ void format_uint(char *buf, uint64_t value, unsigned int base) bt_common_abort(); } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-nonliteral" sprintf(buf_start, spec, value); +#pragma GCC diagnostic pop if (sep_digits) { bt_common_sep_digits(buf_start, digits_per_group, sep); @@ -142,7 +145,10 @@ void format_int(char *buf, int64_t value, unsigned int base) bt_common_abort(); } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-nonliteral" sprintf(buf_start, spec, abs_value); +#pragma GCC diagnostic pop if (sep_digits) { bt_common_sep_digits(buf_start, digits_per_group, sep); diff --git a/src/string-format/format-error.c b/src/string-format/format-error.c index 64b27d12..f6e74390 100644 --- a/src/string-format/format-error.c +++ b/src/string-format/format-error.c @@ -139,9 +139,12 @@ gchar *format_bt_error( i == bt_error_get_cause_count(error) - 1 ? "%s%sERROR%s: " : "%s%sCAUSED BY%s "; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-nonliteral" /* Print prefix */ g_string_append_printf(str, prefix_fmt, codes.bold, codes.fg_bright_red, codes.reset); +#pragma GCC diagnostic pop g_free(error_cause_str); error_cause_str = format_bt_error_cause(cause, columns, diff --git a/tests/param-validation/test_param_validation.c b/tests/param-validation/test_param_validation.c index 62cd858b..fa140104 100644 --- a/tests/param-validation/test_param_validation.c +++ b/tests/param-validation/test_param_validation.c @@ -40,20 +40,19 @@ enum bt_param_validation_status run_test( status = bt_param_validation_validate(params, entries, &validate_error); if (expected_error) { - const char *fmt; - /* We expect a failure. */ ok(status == BT_PARAM_VALIDATION_STATUS_VALIDATION_ERROR, "%s: validation fails", test_name); ok(validate_error, "%s: error string is not NULL", test_name); - fmt = "%s: error string contains expected string"; +#define BT_FMT "%s: error string contains expected string" if (validate_error && strstr(validate_error, expected_error)) { - pass(fmt, test_name); + pass(BT_FMT, test_name); } else { - fail(fmt, test_name); + fail(BT_FMT, test_name); diag("could not find `%s` in `%s`", expected_error, validate_error); } +#undef BT_FMT g_free(validate_error); } else { diff --git a/tests/utils/tap/tap.c b/tests/utils/tap/tap.c index e11dff4f..9f41408b 100644 --- a/tests/utils/tap/tap.c +++ b/tests/utils/tap/tap.c @@ -318,7 +318,7 @@ skip(unsigned int n, const char *fmt, ...) LOCK; va_start(ap, fmt); - if (asprintf(&skip_msg, fmt, ap) == -1) { + if (vasprintf(&skip_msg, fmt, ap) == -1) { skip_msg = NULL; } va_end(ap); -- 2.34.1