-
Notifications
You must be signed in to change notification settings - Fork 567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: move errExit macro into inline function #6217
Conversation
Move most of the `errExit` macro into a new `_errExit` inline function and use the former just to forward arguments to the latter. This reduces the noise in the build output when using `-fanalyzer`, as it causes the `errExit` macro to stop being expanded. For example, the complete output of the following warning in src/firejail/dbus.c is reduced from 243 lines to 141 lines (a ~41% reduction): $ pacman -Q gcc gcc 13.2.1-5 $ ./configure --enable-apparmor --enable-analyzer >/dev/null && make clean >/dev/null && make >/dev/null [...] ../../src/firejail/dbus.c: In function ‘dbus_proxy_start’: ../../src/firejail/dbus.c:311:36: warning: leak of file descriptor ‘dup2(output_fd, 1)’ [CWE-775] [-Wanalyzer-fd-leak] 311 | if (dup2(output_fd, STDOUT_FILENO) != STDOUT_FILENO) [...] ‘dbus_create_user_dir’: event 5 | |../../src/firejail/../include/common.h:42:25: | 42 | #define errExit(msg) do { \ | | ^ | | | | | (5) ...to here ../../src/firejail/dbus.c:239:17: note: in expansion of macro ‘errExit’ | 239 | errExit("asprintf"); | | ^~~~~~~ [...] Relates to netblue30#6190.
a696391
to
f7da0e7
Compare
Note: This also fixes hundreds of (seemingly false positive) warnings when On commit 092bb0a ("build(deps): bump github/codeql-action from 3.24.0 to $ make clean >/dev/null &&
cppcheck --force --error-exitcode=1 --enable=warning,performance \
-i src/firejail/checkcfg.c -i src/firejail/main.c .
Checking src/etc-cleanup/main.c ...
Checking src/etc-cleanup/main.c: __FUNCTION__;__func__...
src/etc-cleanup/main.c:73:3: warning: %s in format string (no. 3) requires 'char *' but the argument type is 'signed int'. [invalidPrintfArgType_s]
errExit("strdup");
^
1/149 files checked 0% done
Checking src/fbuilder/build_bin.c ...
Checking src/fbuilder/build_bin.c: __FUNCTION__;__func__...
src/fbuilder/build_bin.c:108:4: warning: %s in format string (no. 3) requires 'char *' but the argument type is 'signed int'. [invalidPrintfArgType_s]
errExit("asprintf");
^
2/149 files checked 0% done
Checking src/fbuilder/build_fs.c ...
Checking src/fbuilder/build_fs.c: __FUNCTION__;__func__...
src/fbuilder/build_fs.c:118:4: warning: %s in format string (no. 3) requires 'char *' but the argument type is 'signed int'. [invalidPrintfArgType_s]
errExit("asprintf");
^
src/fbuilder/build_fs.c:294:3: warning: %s in format string (no. 3) requires 'char *' but the argument type is 'signed int'. [invalidPrintfArgType_s]
errExit("asprintf");
^
3/149 files checked 1% done
[...]
$ make clean >/dev/null &&
cppcheck -q --force --error-exitcode=1 --enable=warning,performance \
-i src/firejail/checkcfg.c -i src/firejail/main.c . | grep 'warning:' | wc -l
684 On commit 5c65577 ("Merge pull request #6217 from kmk3/build-errexit-func", $ make clean >/dev/null &&
cppcheck --force --error-exitcode=1 --enable=warning,performance \
-i src/firejail/checkcfg.c -i src/firejail/main.c .
Checking src/etc-cleanup/main.c ...
Checking src/etc-cleanup/main.c: __FUNCTION__;__func__...
1/149 files checked 0% done
Checking src/fbuilder/build_bin.c ...
Checking src/fbuilder/build_bin.c: __FUNCTION__;__func__...
2/149 files checked 0% done
Checking src/fbuilder/build_fs.c ...
Checking src/fbuilder/build_fs.c: __FUNCTION__;__func__...
3/149 files checked 1% done
[...]
$ make clean >/dev/null &&
cppcheck -q --force --error-exitcode=1 --enable=warning,performance \
-i src/firejail/checkcfg.c -i src/firejail/main.c . | grep 'warning:' | wc -l
0 CI is currently not affected, as older versions of cppcheck are used. From the runs of commit 092bb0a ("build(deps): bump github/codeql-action from From https://github.com/netblue30/firejail/actions/runs/7964810742/job/21743097070:
From https://github.com/netblue30/firejail/actions/runs/7961492382/job/21732847902:
|
Move most of the
errExit
macro into a new_errExit
inline functionand use the former just to forward arguments to the latter.
This reduces the noise in the build output when using
-fanalyzer
, asit causes the
errExit
macro to stop being expanded.For example, the complete output of the following warning in
src/firejail/dbus.c is reduced from 243 lines to 141 lines (a ~41%
reduction):
Relates to #6190.