Skip to content

Commit

Permalink
dup stdout / stderr if we can
Browse files Browse the repository at this point in the history
Otherwise fall back to open(/dev/foo)
  • Loading branch information
alandekok committed Sep 20, 2023
1 parent 44a9be8 commit 08dbb54
Showing 1 changed file with 14 additions and 1 deletion.
15 changes: 14 additions & 1 deletion src/main/exfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,20 @@ static int exfile_open_mkdir(exfile_t *ef, char const *filename, mode_t permissi
oflag = O_RDWR;
}

fd = open(filename, oflag, permissions);
/*
* Just dup stdout / stderr if it's possible.
*/
if ((default_log.dst == L_DST_STDOUT) &&
(strcmp(filename, "/dev/stdout") == 0)) {
fd = dup(STDOUT_FILENO);

} else if ((default_log.dst == L_DST_STDERR) &&
(strcmp(filename, "/dev/stderr") == 0)) {
fd = dup(STDERR_FILENO);
} else {
fd = open(filename, oflag, permissions);
}

if (fd < 0) {
fr_strerror_printf("Failed to open file %s: %s",
filename, strerror(errno));
Expand Down

2 comments on commit 08dbb54

@vo-va
Copy link

@vo-va vo-va commented on 08dbb54 Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alandekok Hi, this patch will work for me and all I need is to redirect the logs to stdout. So it solves my issue.
But I've noticed that in some cases it still might not work.

I have a shell file that I am using as Docker container entrypoint that starts radiusd with next options.

radiusd -fl stdout; linenlog option filename = /dev/stderr: - no logs from linelog
radiusd -fl stderr; linenlog option filename = /dev/stderr: - no logs at all
radiusd -fxxl stderr; linenlog option filename = /dev/stderr: - no logs at all
radiusd -fxxl stdout; linenlog option filename = /dev/stderr: - error: rlm_linelog: Failed to open /dev/stderr: Permission denied

I guess only the last case is an issue and using -fl stderr is not expected.

I am not sure how properly to fix this in exfile.c. and I've planned to submit PR for rlm_linelog that will just use fprintf(). But now after seeing this commit, I am not sure if my plan is the best option.

@alandekok
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't use fprintf() in rlm_linelog. It will still use stdout / stderr, and that has all of the same problems as just using write().'

I don't know why it doesn't write to stderr. I'd have to look into it, and I don't have time right now. The simplest solution is just to use stdout.

Please sign in to comment.