Fix typos Run `typos -w` [1] on the repository, and cherry-pick the good changes. [1] https://github.com/crate-ci/typos Change-Id: I995cf7d851cd7ff40e47767ffe14c03fbd2d23bb Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/11426 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Fix REUSE licensing/copyright issues in `tests/utils` This patch makes `reuse lint` happy for the files in `tests/utils`. Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com> Change-Id: I472089852d8840653c0a4a2350a4d4981329f43e Reviewed-on: https://review.lttng.org/c/babeltrace/+/11385 Tested-by: jenkins <jenkins@lttng.org> CI-Build: Michael Jeanson <mjeanson@efficios.com> Reviewed-by: Michael Jeanson <mjeanson@efficios.com>
configure: re-enable '-Wunused-parameter' This warning is part of '-Wextra' in GCC. Change-Id: Ifb514a41cb9c62cf703d415eeb2ccefc331dee77 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/7559 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
tests: add extern "C" to tap.h This makes it possible to use it from C++ programs. Change-Id: Ic111f493b0b048b8e491b824af293f0742bda35a Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/10067 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org>
configure: remove -Wno-format-nonliteral 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 <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/3233 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org>
Move to kernel style SPDX license identifiers The SPDX identifier is a legally binding shorthand, which can be used instead of the full boiler plate text. See https://spdx.org/ids-how for details. Change-Id: I7c25a3bc48ee328500a604cb276877d4cadfa997 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/3227 CI-Build: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
common: cast arguments to character classification functions to unsigned char We get failures of this type on the cygwin CI machine: 15:28:20 common.c: In function `bt_common_string_is_printable`: 15:28:20 common.c:786:16: error: array subscript has type `char` [-Werror=char-subscripts] 15:28:20 786 | if (!isprint(*ch) && *ch != '\n' && *ch != '\r' && 15:28:20 | ^~~ This error only pops up on some platforms that have isprint implemented using a lookup table. This table is indexed using `*ch`, which is a char. And because char is signed on some platforms, gcc warns that this is dangerous: we could access the array with a negative index, which would yield unexpected results. This is on purpose in newlib (the libc used by cygwin, apparently), see this comment: https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/ctype.h;h=a0009af17485acc3d70586a0051269a7a9c350d5;hb=HEAD#l78 The Linux man page for isprint also mentions it: The standards require that the argument c for these functions is either EOF or a value that is representable in the type unsigned char. If the argument c is of type char, it must be cast to unsigned char, as in the following example: char c; ... res = toupper((unsigned char) c); This is necessary because char may be the equivalent of signed char, in which case a byte where the top bit is set would be sign extended when converting to int, yielding a value that is outside the range of unsigned char. Add casts to unsigned char to fix the various instances of this error. Change-Id: Ice2305490997f595c6f5140a8be2abaa7fd1d8f6 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/3194 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> CI-Build: Michael Jeanson <mjeanson@efficios.com> Tested-by: jenkins <jenkins@lttng.org>
Fix -Wsuggest-attribute warnings plan_skip_all unconditionally exits, so it can be marked as noreturn. Fixes: CC tap.lo /home/smarchi/src/babeltrace/tests/utils/tap/tap.c: In function ‘plan_skip_all’: /home/smarchi/src/babeltrace/tests/utils/tap/tap.c:231:1: error: function might be candidate for attribute ‘noreturn’ [-Werror=suggest-attribute=noreturn] plan_skip_all(const char *reason) ^~~~~~~~~~~~~ Change-Id: I740aa065c888c53a7be880a9e6e68e6f12b1d42d Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/2259 Tested-by: jenkins <jenkins@lttng.org> Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
configure: allow adding compiler-specific warning flags Goal ---- The goal of this patch is to add a system where we can provide a bunch of warning flags, and it will check if the current compiler supports each of them individually. This is inspired by how GDB's configure does: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/warning.m4;h=c9e64a1836a8e01339ca4e3c3d689657ff9ef92c;hb=5c49f2cd78c69d50bc7c7119596a226f05939d06#l19 GDB's macro is homegrown, but unfortunately we can't just copy it because of the license (it's GPLv3). We could write our own macro that does the same thing, it's not terribly complicated. But rather than writing our own macro... Implementation -------------- There is a macro in the Autoconf Archive, AX_COMPILER_FLAGS, which does pretty much that. You can pass it a list of flags and it will only keep the ones supported by the compiler. However, it also provides a bunch of arbitrary warning flags of its own. Some of them we want, some of them we probably don't want, and some of them we want but the codebase is not ready for them yet. To keep this patch reasonable, I chose to pass all the -Wno-* flags needed to disable the warnings we have currently. Over time, we can gradually fix the warnings and remove them from that list. The documentation for AX_COMPILER_FLAGS specifically says: The set of base and enabled flags can be augmented using the EXTRA-*-CFLAGS and EXTRA-*-LDFLAGS variables, which are tested and appended to the output variable if –enable-compile-warnings is not "no". Flags should not be disabled using these arguments, as the entire point of AX_COMPILER_FLAGS is to enforce a consistent set of useful compiler warnings on code, using warnings which have been chosen for low false positive rates. If a compiler emits false positives for a warning, a #pragma should be used in the code to disable the warning locally. However, there's no way we're going to add so many pragmas, some of them for warnings we might not even want. So I took the liberty to use the EXTRA-YES-CFLAGS to pass the arguments to disable those warnings. Notes ----- (1) I have fixed the few occurences where we pass a const pointer as a non-const pointer (e.g.: error: passing argument 2 of ‘g_spawn_sync’ from incompatible pointer type), because with gcc 4.8 there is no way of silencing that warning. (2) As explained in the comment in configure.ac, I have moved the glib sizeof(size_t) check before the call to AX_COMPILER_FLAGS. This is so the sizeof(size_t) check is not affected by the strict warning flags enabled by AX_COMPILER_FLAGS. In practice, the problem is that the program generated by AC_LANG_PROGRAM defines main as "int main()" instead of "int main(void)", causing a -Wold-style-declaration warning. (3) As explained in the comment in configure.ac, I have explicitly enabled -Wold-style-declaration. I have fixed the offender. (4) This macro adds the following arguments to configure: --enable-compile-warnings=[no/yes/error] Enable compiler warnings and errors --disable-Werror Unconditionally make all compiler warnings non-fatal (5) The AX_COMPILER_FLAGS macro also provides some useful LDFLAGS. However, it provides -Wl,--no-as-needed, and we specifically use -Wl,--as-needed when building babeltrace2.exe. I chose not to use the LDFLAGS from AX_COMPILER_FLAGS for now, by fear that it will break things in a subtle way. Alternatives ------------ If this is deemed too invasive, alternatives would be to: - write our own macro, like the GDB one - copy the AX_COMPILER_FLAGS and modify it to only keep what we want Change-Id: Ic3e292743a3eb96d786372cd72bedbfbc5986cd0 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/2257 Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Michael Jeanson <mjeanson@efficios.com> Tested-by: jenkins <jenkins@lttng.org>
tests: rename PRINT_FORMAT to TAP_PRINTF_FORMAT Use a namespace, so we can define our own for the rest of babeltrace (will be called BT_PRINTF_FORMAT) without colliding. Change-Id: I03f5d4cf050f94de294bb179450876b300d53b91 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/2165 Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Fix: format string warnings on mingw Since we have added format string attributes in the tap.h file, we get these diagnostics on Windows: CC test_bitfield.o test_bitfield.c: In function 'run_test_unsigned_write': test_bitfield.c:51:36: error: unknown conversion type character 'l' in format [-Werror=format=] 51 | #define DIAG_FMT_STR(val_type_fmt) "Failed reading value written \"%s\"-wise, with start=%i" \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test_bitfield.c:51:36: note: in definition of macro 'DIAG_FMT_STR' 51 | #define DIAG_FMT_STR(val_type_fmt) "Failed reading value written \"%s\"-wise, with start=%i" \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test_bitfield.c:173:8: note: in expansion of macro 'check_result' 173 | if (check_result(src_ui, readval, target.c, unsigned char, | ^~~~~~~~~~~~ test_bitfield.c:175:9: note: format string is defined here 175 | "%llX")) { | ^ By default, printf on Windows doesn't know about the `ll` format string length modifier (as in %llx). However, we build Babeltrace with the __USE_MINGW_ANSI_STDIO macro defined, which makes mingw use a replacement printf implementation that knows about `ll`. When we define our own functions with a format attribute, we must then use __MINGW_PRINTF_FORMAT instead of `printf`. __MINGW_PRINTF_FORMAT will take the right value, depending on __USE_MINGW_ANSI_STDIO, so it will validate the format string according to the selected printf implementation. Change-Id: I89b194e915250a0e0a617817a1cd10fedb156639 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/2161 Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Tested-by: jenkins <jenkins@lttng.org>
tests: constify format strings in tap.h Format strings are typically read-only data (literal strings), and they are not modified by the functions receiving them, so it makes no sense for them not to be const. This helps avoid some "passing value to function foo discards const qualifier" kind of warnings. Change-Id: Ibaaaf5690bed21e1d541f1b676627dfd70541219 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/2130 Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins <jenkins@lttng.org>
tests: add format attributes to functions receiving format strings in tap.h This makes the compiler validate the format strings against the types of the passed arguments. It found a few (minor) issues, which are also fixed by this patch. Note that in test_bitfield.c, the format string now includes "0x" before printing the number in hexadecimal, to avoid any confusion when the user reads it. Change-Id: I07cac88aa3cdd445d79f2c12bc0f9333f6a768a9 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/2129 Tested-by: jenkins <jenkins@lttng.org>
tests: build system spring cleanup * Remove AC_CONFIG_FILES generated files from check_SCRIPTS and EXTRA_DIST since they are already included. * Remove useless conditionnals on AC_CONFIG_FILES generated files since they are always included. * Reduce the number of mostly unused Makefiles * Remove execute permission on data files Change-Id: I4e82bac01bf2033409c67508a4d60127a0e77f14 Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Reviewed-on: https://review.lttng.org/c/babeltrace/+/1350 CI-Build: Francis Deslauriers <francis.deslauriers@efficios.com> Tested-by: jenkins Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Re-implement BT_ASSERT without using the assert macro BT_ASSERT is currently defined using the assert macro. If NDEBUG is inadvertently defined, BT_ASSERT has no effect. This is quite unfortunate, since a user who has defined BABELTRACE_DEBUG_MODE to 1 probably wants BT_ASSERT to be effective. This problem was encountered while building the Python bindings, where the distutils native code builder is passing -DNDEBUG to the compiler. The BT_ASSERT macro usages in these files were found to be ineffective. This patch avoids this situation by making BT_ASSERT call our own handler if the assertion fails. This also has the advantage of letting us personalize the behavior on assertion failures. The presence of the so-called Lenny Face having absolutely no regards for the furniture is a good example of this (and was a requirement coming from Philippe Proulx). Removing the inclusion of assert.h in assert-internal.h revealed a few spots that use the assert macro without including assert.h, so they were adjusted accordingly. Signed-off-by: Simon Marchi <simon.marchi@efficios.com>