Re: [PATCH v2 1/6] conf, passt: Don't try to log to stderr after we close it

On Wed, 19 Jun 2024 12:14:53 +1000 David Gibson wrote:

On Tue, Jun 18, 2024 at 09:14:22AM +0200, Stefano Brivio wrote:

...

If we don't run in foreground, we close standard error as we daemonise, so it makes no sense to check if the controlling terminal is an interactive terminal or if --force-stderr was given, to decide if we want to log to standard error.

Make --force-stderr depend on --foreground.

Signed-off-by: Stefano Brivio --- conf.c | 3 +++ passt.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/conf.c b/conf.c index 94b3ed6..dbdbb62 100644 --- a/conf.c +++ b/conf.c @@ -1693,6 +1693,9 @@ void conf(struct ctx *c, int argc, char **argv)

conf_ugid(runas, &uid, &gid);

+ if (!c->foreground && c->force_stderr) + die("Can't log to standard error if not running in foreground"); + if (logfile) { logfile_init(c->mode == MODE_PASTA ? "pasta" : "passt", logfile, logsize); diff --git a/passt.c b/passt.c index a5e2c5a..aa9648a 100644 --- a/passt.c +++ b/passt.c @@ -302,7 +302,7 @@ int main(int argc, char **argv) if (isolate_prefork(&c)) die("Failed to sandbox process, exiting");

- if (!c.force_stderr && !isatty(fileno(stderr))) + if (!c.foreground || (!c.force_stderr && !isatty(fileno(stderr)))) __openlog(log_name, 0, LOG_DAEMON);

Hm.. kind of preexisting, but shouldn't we still skip the __openlog() if we have a logfile?

Ah, true. I would add this as a separate patch.

Or make __openlog() open either the syslog or the logfile as appropriate (but in that case we should rename it not to look like openlog(3)).

I would rather keep __openlog() as openlog() implementation, because the semantics are well specified like this. We just need another function, or even a direct setting, for LOG_PERROR (or get rid of that flag, internally?). -- Stefano