[GH-ISSUE #5873] Logging macros and improving error/warning messages #3116

Open
opened 2026-05-05 09:45:10 -06:00 by gitea-mirror · 1 comment
Owner

Originally created by @kmk3 on GitHub (Jun 28, 2023).
Original GitHub issue: https://github.com/netblue30/firejail/issues/5873

Background

This PR was opened to improve errExit() messages:

However, errExit is mostly only used when basic libc calls fail (as it uses
perror(3p) to print the errno message); most warning/error messages are
printed manually and do not contain the file path. Example:

Error: invalid sandbox name

Proposal

So how about adding more logging macros?

Examples:

log_trace("starting foo");
// TRACE src/firejail/util.c:123 x(): starting foo

log_debug("foo is %d, foo);
// DEBUG src/firejail/util.c:123 check_foo(): foo is 1

log_info("Reading profile %s", profile);
log_info("firejail version 0.9.73");
// INFO  profile_read(): Reading profile /etc/firejail/foo.profile
// INFO  profile_read(): Reading profile /etc/firejail/globals.local
// [...]
// INFO  main(): firejail version 0.9.73

log_warning("failed to open file: %s", foo);
// WARN  src/firejail/util.c:123 check_foo(): failed to open file: /foo/bar

log_error("invalid --foo: %s", foo);
// ERROR src/firejail/main.c:123 main(): invalid --foo: aaa-
// (program exits)

log_fatal("malloc"); // this is basically errExit()
// FATAL src/firejail/util.c:123 check_foo(): malloc: Cannot allocate memory
// (program exits)

Main differences between functions:

  • log_info does not include the file path (for readability)
  • log_error and log_fatal exit the program
  • log_fatal uses perror(3p) to print the reason (like errExit)

I'm not sure about the format of info and below, so the first steps would be to
replace manual warning/error messages (fprintf calls) with the log functions
so that they include the file path and function in the message; how the
functions operate internally (format, destination, etc) could be improved
later.

Also add --loglevel= to control which messages get printed:

  • --loglevel=[trace|debug|info|warning|error|fatal]
  • --quiet would map to --loglevel=error
  • --debug would map to --loglevel=debug or --loglevel=trace

The default would be info.

As for the last item, considering the amount of things that --debug prints
(such as the assembly dumps of seccomp filters), it seems more like a trace
option, so maybe it would make sense to change the messages that are currently
inside if (arg_debug) to use log_trace() and then deprecate --debug.

Relates to:

Originally created by @kmk3 on GitHub (Jun 28, 2023). Original GitHub issue: https://github.com/netblue30/firejail/issues/5873 ### Background This PR was opened to improve `errExit()` messages: * <https://github.com/netblue30/firejail/pull/5871> However, `errExit` is mostly only used when basic libc calls fail (as it uses `perror(3p)` to print the errno message); most warning/error messages are printed manually and do not contain the file path. Example: > Error: invalid sandbox name ### Proposal So how about adding more logging macros? Examples: ```C log_trace("starting foo"); // TRACE src/firejail/util.c:123 x(): starting foo log_debug("foo is %d, foo); // DEBUG src/firejail/util.c:123 check_foo(): foo is 1 log_info("Reading profile %s", profile); log_info("firejail version 0.9.73"); // INFO profile_read(): Reading profile /etc/firejail/foo.profile // INFO profile_read(): Reading profile /etc/firejail/globals.local // [...] // INFO main(): firejail version 0.9.73 log_warning("failed to open file: %s", foo); // WARN src/firejail/util.c:123 check_foo(): failed to open file: /foo/bar log_error("invalid --foo: %s", foo); // ERROR src/firejail/main.c:123 main(): invalid --foo: aaa- // (program exits) log_fatal("malloc"); // this is basically errExit() // FATAL src/firejail/util.c:123 check_foo(): malloc: Cannot allocate memory // (program exits) ``` Main differences between functions: * `log_info` does not include the file path (for readability) * `log_error` and `log_fatal` exit the program * `log_fatal` uses `perror(3p)` to print the reason (like `errExit`) I'm not sure about the format of info and below, so the first steps would be to replace manual warning/error messages (`fprintf` calls) with the log functions so that they include the file path and function in the message; how the functions operate internally (format, destination, etc) could be improved later. Also add `--loglevel=` to control which messages get printed: * `--loglevel=[trace|debug|info|warning|error|fatal]` * `--quiet` would map to `--loglevel=error` * `--debug` would map to `--loglevel=debug` or `--loglevel=trace` The default would be `info`. As for the last item, considering the amount of things that `--debug` prints (such as the assembly dumps of seccomp filters), it seems more like a trace option, so maybe it would make sense to change the messages that are currently inside `if (arg_debug)` to use `log_trace()` and then deprecate `--debug`. Relates to: * <https://github.com/netblue30/firejail/issues/2322> * <https://github.com/netblue30/firejail/pull/3325> * <https://github.com/netblue30/firejail/issues/3354> * <https://github.com/netblue30/firejail/issues/3371> * <https://github.com/netblue30/firejail/issues/3749> * <https://github.com/netblue30/firejail/issues/4275> * <https://github.com/netblue30/firejail/pull/5871>
gitea-mirror added the
enhancement
label 2026-05-05 09:45:10 -06:00
Author
Owner

@kmk3 commented on GitHub (Aug 15, 2025):

As mentioned in #3325, it may be unclear that some error/warning messages come
from firejail, which could be resolved by adding a prefix with the program name
(and maybe PID).

Example:

firejail(<PID>): (message)

This could go in errExit / fwarning.

Note: Something similar is already used in preproc.c:

pid=<PID>: (message)

Which was added in the following PR:

Also, something like errExitx and fwarningx (similarly to err(3) /
errx(3)) could be added, in the cases where strerror(errno) is not intended
to be printed.

<!-- gh-comment-id:3191343620 --> @kmk3 commented on GitHub (Aug 15, 2025): As mentioned in #3325, it may be unclear that some error/warning messages come from firejail, which could be resolved by adding a prefix with the program name (and maybe PID). Example: ``` firejail(<PID>): (message) ``` This could go in `errExit` / `fwarning`. Note: Something similar is already used in preproc.c: ``` pid=<PID>: (message) ``` Which was added in the following PR: * #6750 Also, something like `errExitx` and `fwarningx` (similarly to `err(3)` / `errx(3)`) could be added, in the cases where `strerror(errno)` is not intended to be printed.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/firejail#3116
No description provided.