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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]

On Wed, Jun 19, 2024 at 10:34:48AM +0200, Stefano Brivio wrote:
> On Wed, 19 Jun 2024 12:14:53 +1000
> David Gibson <david@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@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.

I concur.

> We just need another function, or even a direct setting, for LOG_PERROR
> (or get rid of that flag, internally?).

I feel like our needs for when we log to stderr are specific enough
that it's simpler to just not use LOG_PERROR at all, and handle the
printing to stderr ourselves.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]