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 <david(a)gibson.dropbear.id.au> 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 <sbrivio(a)redhat.com>
 ---
  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