From 47350c5fb64e7c76e071a928bf9251dfe79d3ab3 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Wed, 14 Apr 2021 13:17:22 +0200 Subject: meson: simplify the BUILD_MODE conditional Using a enum is all nice and generic, but at this point it seems unlikely that we'll add further build modes. But having an enum means that we need to include the header file with the enumeration whenerever the conditional is used. I want to use the conditional in log.h, which makes it hard to avoid circular imports. --- meson.build | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'meson.build') diff --git a/meson.build b/meson.build index fa929370bd..7d9f2d3d4c 100644 --- a/meson.build +++ b/meson.build @@ -38,8 +38,8 @@ relative_source_path = run_command('realpath', project_source_root).stdout().strip() conf.set_quoted('RELATIVE_SOURCE_PATH', relative_source_path) -conf.set('BUILD_MODE', 'BUILD_MODE_' + get_option('mode').to_upper(), - description : 'tailor build to development or release builds') +conf.set10('BUILD_MODE_DEVELOPER', get_option('mode') == 'developer', + description : 'tailor build to development or release builds') want_ossfuzz = get_option('oss-fuzz') want_libfuzzer = get_option('llvm-fuzz') @@ -1117,7 +1117,7 @@ else libcurl = [] endif conf.set10('HAVE_LIBCURL', have) -conf.set10('CURL_NO_OLDIES', get_option('mode') == 'developer') +conf.set10('CURL_NO_OLDIES', conf.get('BUILD_MODE_DEVELOPER') == 1) want_libidn = get_option('libidn') want_libidn2 = get_option('libidn2') @@ -3640,7 +3640,7 @@ if dbus_docs.length() > 0 '@INPUT@'], input : dbus_docs) - if conf.get('BUILD_MODE') == 'BUILD_MODE_DEVELOPER' + if conf.get('BUILD_MODE_DEVELOPER') == 1 test('dbus-docs-fresh', update_dbus_docs_py, args : ['--build-dir=@0@'.format(project_build_root), -- cgit v1.2.3 From a626cb15c0a029270779887dc3bc12e2dcadd273 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Wed, 14 Apr 2021 15:55:16 +0200 Subject: basic/log: assert that 0 is not passed as errno, except in test code Let's assert if we ever happen to pass 0 to one of the log functions. With the preceding commit to return -EIO from log_*(), passing 0 wouldn't affect the return value any more, but it is still most likely an error. The unit test code is an exception: we fairly often pass the return value to print it, before checking what it is. So let's assert that we're not passing 0 in non-test code. As with the previous check for %m, this is only done in developer mode. We are depending on external code setting errno correctly for us, which might not always be true, and which we can't test, so we shouldn't assert, but just handle this gracefully. I did a bunch of greps to try to figure out if there are any places where we're passing 0 on purpose, and couldn't find any. The one place that failed in tests is adjusted. About "zerook" in the name: I wanted the suffix to be unambiguous. It's a single "word" because each of the words in log_full_errno is also meaningful, and having one term use two words would be confusing. --- meson.build | 22 +++++++++++++--------- src/basic/log.h | 19 ++++++++++++++++--- src/shared/bus-wait-for-jobs.c | 3 ++- src/test/test-bpf-foreign-programs.c | 2 +- 4 files changed, 32 insertions(+), 14 deletions(-) (limited to 'meson.build') diff --git a/meson.build b/meson.build index 7d9f2d3d4c..8b4458d205 100644 --- a/meson.build +++ b/meson.build @@ -3318,11 +3318,15 @@ custom_target( '} >@OUTPUT@'], build_by_default : true) -# We intentionally do not do inline initializations with definitions for -# a bunch of _cleanup_ variables in tests, to ensure valgrind is triggered. -# This triggers a lot of maybe-uninitialized false positives when the -# combination of -O2 and -flto is used. Suppress them. -no_uninit = '-O2' in get_option('c_args') and '-flto=auto' in get_option('c_args') ? cc.first_supported_argument('-Wno-maybe-uninitialized') : [] +test_cflags = ['-DTEST_CODE=1'] +# We intentionally do not do inline initializations with definitions for a +# bunch of _cleanup_ variables in tests, to ensure valgrind is triggered if we +# use the variable unexpectedly. This triggers a lot of maybe-uninitialized +# false positives when the combination of -O2 and -flto is used. Suppress them. +if '-O2' in get_option('c_args') and '-flto=auto' in get_option('c_args') + test_cflags += cc.first_supported_argument('-Wno-maybe-uninitialized') +endif + foreach tuple : tests sources = tuple[0] link_with = tuple.length() > 1 and tuple[1].length() > 0 ? tuple[1] : [libshared] @@ -3331,7 +3335,7 @@ foreach tuple : tests condition = tuple.length() > 4 ? tuple[4] : '' type = tuple.length() > 5 ? tuple[5] : '' defs = tuple.length() > 6 ? tuple[6] : [] - defs += no_uninit + defs += test_cflags parallel = tuple.length() > 7 ? tuple[7] : true timeout = 30 @@ -3399,7 +3403,7 @@ exe = executable( 'test-libudev-sym', test_libudev_sym_c, include_directories : libudev_includes, - c_args : ['-Wno-deprecated-declarations'] + no_uninit, + c_args : ['-Wno-deprecated-declarations'] + test_cflags, link_with : [libudev], build_by_default : want_tests != 'false', install : install_tests, @@ -3412,7 +3416,7 @@ exe = executable( 'test-libudev-static-sym', test_libudev_sym_c, include_directories : libudev_includes, - c_args : ['-Wno-deprecated-declarations'] + no_uninit, + c_args : ['-Wno-deprecated-declarations'] + test_cflags, link_with : [install_libudev_static], build_by_default : want_tests != 'false' and static_libudev_pic, install : install_tests and static_libudev_pic, @@ -3453,7 +3457,7 @@ foreach tuple : fuzzers include_directories : [incs, include_directories('src/fuzz')], link_with : link_with, dependencies : dependencies, - c_args : defs, + c_args : defs + test_cflags, link_args: link_args, install : false, build_by_default : fuzz_tests or fuzzer_build) diff --git a/src/basic/log.h b/src/basic/log.h index f7785b016b..9a57fcd0f1 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -189,7 +189,7 @@ void log_assert_failed_return( log_dispatch_internal(level, error, PROJECT_FILE, __LINE__, __func__, NULL, NULL, NULL, NULL, buffer) /* Logging with level */ -#define log_full_errno(level, error, ...) \ +#define log_full_errno_zerook(level, error, ...) \ ({ \ int _level = (level), _e = (error); \ _e = (log_get_max_level() >= LOG_PRI(_level)) \ @@ -198,17 +198,30 @@ void log_assert_failed_return( _e < 0 ? _e : -EIO; \ }) +#if BUILD_MODE_DEVELOPER && !defined(TEST_CODE) +# define ASSERT_NON_ZERO(x) assert((x) != 0) +#else +# define ASSERT_NON_ZERO(x) +#endif + +#define log_full_errno(level, error, ...) \ + ({ \ + int _error = (error); \ + ASSERT_NON_ZERO(_error); \ + log_full_errno_zerook(level, _error, __VA_ARGS__); \ + }) + #define log_full(level, fmt, ...) \ ({ \ if (BUILD_MODE_DEVELOPER) \ assert(!strstr(fmt, "%m")); \ - (void) log_full_errno((level), 0, fmt, ##__VA_ARGS__); \ + (void) log_full_errno_zerook(level, 0, fmt, ##__VA_ARGS__); \ }) int log_emergency_level(void); /* Normal logging */ -#define log_debug(...) log_full_errno(LOG_DEBUG, 0, __VA_ARGS__) +#define log_debug(...) log_full_errno_zerook(LOG_DEBUG, 0, __VA_ARGS__) #define log_info(...) log_full(LOG_INFO, __VA_ARGS__) #define log_notice(...) log_full(LOG_NOTICE, __VA_ARGS__) #define log_warning(...) log_full(LOG_WARNING, __VA_ARGS__) diff --git a/src/shared/bus-wait-for-jobs.c b/src/shared/bus-wait-for-jobs.c index e66c8beafa..8458fe8684 100644 --- a/src/shared/bus-wait-for-jobs.c +++ b/src/shared/bus-wait-for-jobs.c @@ -306,7 +306,8 @@ int bus_wait_for_jobs(BusWaitForJobs *d, bool quiet, const char* const* extra_ar if (q < 0 && r == 0) r = q; - log_debug_errno(q, "Got result %s/%m for job %s", d->result, d->name); + log_full_errno_zerook(LOG_DEBUG, q, + "Got result %s/%m for job %s", d->result, d->name); } d->name = mfree(d->name); diff --git a/src/test/test-bpf-foreign-programs.c b/src/test/test-bpf-foreign-programs.c index e703924077..666317e520 100644 --- a/src/test/test-bpf-foreign-programs.c +++ b/src/test/test-bpf-foreign-programs.c @@ -293,7 +293,7 @@ int main(int argc, char *argv[]) { r = cg_all_unified(); if (r <= 0) - return log_tests_skipped_errno(r, "Unified hierarchy is required, skipping."); + return log_tests_skipped("Unified hierarchy is required, skipping."); r = enter_cgroup_subroot(NULL); if (r == -ENOMEDIUM) -- cgit v1.2.3