Skip to content

Commit 8fad4af

Browse files
committed
logger.c: Don't log to STDOUT if it's redirected to /dev/null.
Previously, we assumed that if the BBS was running in the foreground, it was connected to a TTY, such as a PTY, under screen, etc. However, under systemd, the BBS is run in the foreground but not necessarily with a TTY. This is actually okay, except if StandardOutput=null, then STDOUT is redirected to /dev/null, so any logs written there are just discarded. This is actually okay, too (attempting to write to /dev/null obviously won't cause an issue), but for a slight efficiency boost, we now detect this on startup so we can skip logging work that is specific to the foreground console later on.
1 parent 5c5c52b commit 8fad4af

File tree

1 file changed

+35
-5
lines changed

1 file changed

+35
-5
lines changed

bbs/logger.c

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,18 +144,48 @@ int bbs_log_init(int nofork)
144144
return -1;
145145
}
146146

147+
/* Open the log file */
147148
snprintf(logfile, sizeof(logfile), "%s/%s", BBS_LOG_DIR, "bbs.log");
148-
logstdout = nofork;
149-
stdoutavailable = nofork;
150-
151149
if (!(logfp = fopen(logfile, "a"))) {
152150
fprintf(stderr, "Unable to open log file: %s (%s)\n", logfile, strerror(errno));
153151
return -1;
154152
}
155153
fprintf(logfp, "=== BBS logger initialization (pid %d) ===\n", bbs_gettid());
156-
if (logstdout) {
154+
155+
stdoutavailable = nofork;
156+
if (stdoutavailable) {
157157
fflush(stdout);
158158
}
159+
160+
/* Refine our determination of whether we can actually log to STDOUT or not. */
161+
if (stdoutavailable && !isatty(STDOUT_FILENO)) {
162+
/* Even if we didn't daemonize, that doesn't mean we can actually write logs to STDOUT.
163+
* We could check if isatty(STDOUT), but that isn't necessarily what we want.
164+
* For example, under systemd, if StandardOutput isn't null, we may want to allow systemd to collect
165+
* logs sent to STDOUT.
166+
* However, if StandardOutput=null, then STDOUT and friends are redirected to /dev/null,
167+
* and then there is literally no point in logging anything there.
168+
* It won't cause an error, but it's extra work for nothing.
169+
* So, at startup, check if STDOUT is actually connected to something "useful" (whether it's a TTY or not),
170+
* or if it's /dev/null. If the latter, then disable the ability to log to STDOUT. */
171+
struct stat stdout, devnull;
172+
if (fstat(STDOUT_FILENO, &stdout) || !S_ISCHR(stdout.st_mode)) {
173+
bbs_warning("fstat failed: %s\n", strerror(errno));
174+
} else if (stat("/dev/null", &devnull)) {
175+
bbs_warning("stat failed: %s\n", strerror(errno));
176+
} else {
177+
/* If STDOUT and /dev/null are both devices with the same inode,
178+
* then the former was redirected to the latter. */
179+
if (stdout.st_dev == devnull.st_dev && stdout.st_ino == devnull.st_ino) {
180+
bbs_debug(1, "Disabling logging to STDOUT, since it's redirected to /dev/null\n");
181+
/* Okay, fine. Now we can save a little work for every log message. */
182+
stdoutavailable = 0;
183+
}
184+
}
185+
}
186+
187+
logstdout = stdoutavailable; /* Default */
188+
159189
if (bbs_cli_register_multiple(cli_commands_logger)) {
160190
return -1;
161191
}
@@ -165,7 +195,7 @@ int bbs_log_init(int nofork)
165195
int bbs_set_stdout_logging(int enabled)
166196
{
167197
if (enabled && !stdoutavailable) {
168-
bbs_debug(1, "Can't enable logging to stdout when daemonized\n");
198+
bbs_debug(1, "Can't enable logging to stdout when daemonized or when STDOUT is redirected to /dev/null\n");
169199
return -1;
170200
}
171201
logstdout = enabled;

0 commit comments

Comments
 (0)