Re: vmd: simplify fatal/fatalx errno handling

2016-10-17 Thread Reyk Floeter
On Sun, Oct 16, 2016 at 02:55:39PM -0700, Philip Guenther wrote:
> 
> Instead of using errno as a hidden argument to vfatal(), make it an 
> _actual_ argument named 'code', ala the errc/warnc family, and rename it 
> to vfatalc() to match the pattern set.
> 
> ok?
> 

OK, makes sense

please sync as mentioned by benno and me.

> 
> Index: log.c
> ===
> RCS file: /data/src/openbsd/src/usr.sbin/vmd/log.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 log.c
> --- log.c 12 Oct 2016 11:47:34 -  1.3
> +++ log.c 16 Oct 2016 21:17:40 -
> @@ -165,11 +165,10 @@ log_debug(const char *emsg, ...)
>  }
>  
>  static void
> -vfatal(const char *emsg, va_list ap)
> +vfatalc(int code, const char *emsg, va_list ap)
>  {
>   static char s[BUFSIZ];
>   const char  *sep;
> - int  saved_errno = errno;
>  
>   if (emsg != NULL) {
>   (void)vsnprintf(s, sizeof(s), emsg, ap);
> @@ -178,9 +177,9 @@ vfatal(const char *emsg, va_list ap)
>   s[0] = '\0';
>   sep = "";
>   }
> - if (saved_errno)
> + if (code)
>   logit(LOG_CRIT, "%s: %s%s%s",
> - log_procname, s, sep, strerror(saved_errno));
> + log_procname, s, sep, strerror(code));
>   else
>   logit(LOG_CRIT, "%s%s%s", log_procname, sep, s);
>  }
> @@ -191,7 +190,7 @@ fatal(const char *emsg, ...)
>   va_list ap;
>  
>   va_start(ap, emsg);
> - vfatal(emsg, ap);
> + vfatalc(errno, emsg, ap);
>   va_end(ap);
>   exit(1);
>  }
> @@ -201,9 +200,8 @@ fatalx(const char *emsg, ...)
>  {
>   va_list ap;
>  
> - errno = 0;
>   va_start(ap, emsg);
> - vfatal(emsg, ap);
> + vfatalc(0, emsg, ap);
>   va_end(ap);
>   exit(1);
>  }
> 

-- 



Re: vmd: simplify fatal/fatalx errno handling

2016-10-17 Thread Reyk Floeter

On Sun, Oct 16, 2016 at 08:03:05PM -0600, Theo de Raadt wrote:
> > On Sun, Oct 16, 2016 at 02:55:39PM -0700, Philip Guenther wrote:
> > > 
> > > Instead of using errno as a hidden argument to vfatal(), make it an 
> > > _actual_ argument named 'code', ala the errc/warnc family, and rename it 
> > > to vfatalc() to match the pattern set.
> > > 
> > > ok?
> > > 
> > > Philip Guenther
> > > 
> > 
> > Isn't this code used elsewhere too?
> 
> benno is in the process of unifying all the log.c versions, as much
> as possible.
> 

benno is working on a totally different variant of log.c that is found
in the routing daemons.  I talked with him at g2k16 about it and we'll
eventually try to sync them all together once he is done with that,
but he will first move all the daemon-/protocol-specific stuff out of
the other log.c's.

I unified my versions long ago, and there identical in at least:

usr.sbin/httpd
usr.sbin/ntpd
usr.sbin/relayd
usr.sbin/snmpd
usr.sbin/switchd
usr.sbin/vmd
sbin/iked

$ diff -Nup -I'\$OpenBSD' usr.sbin/vmd/log.c usr.sbin/ntpd/log.c

I think Philip's code is OK and syncing it with the other daemons is
as easy as copying vmd's updated version to all these directories and
committing them at once.

Reyk



Re: vmd: simplify fatal/fatalx errno handling

2016-10-17 Thread Sebastian Benoit
Theo de Raadt(dera...@openbsd.org) on 2016.10.16 20:03:05 -0600:
> > On Sun, Oct 16, 2016 at 02:55:39PM -0700, Philip Guenther wrote:
> > > 
> > > Instead of using errno as a hidden argument to vfatal(), make it an 
> > > _actual_ argument named 'code', ala the errc/warnc family, and rename it 
> > > to vfatalc() to match the pattern set.
> > > 
> > > ok?

i dont consider the use of errno to be a problem here, the same is done in
log_warn() and its kind of consistent with warn(3) etc.

> > > Philip Guenther
> > > 
> > 
> > Isn't this code used elsewhere too?

this variant of log.c is in sync in iked, httpd, relayd, snmpd, vmd and
quite different to the other ones.

if this change goes in here, please sync all these.

> benno is in the process of unifying all the log.c versions, as much
> as possible.

i'll keep a pointer to this thread to consider later.



Re: vmd: simplify fatal/fatalx errno handling

2016-10-16 Thread Theo de Raadt
> On Sun, Oct 16, 2016 at 02:55:39PM -0700, Philip Guenther wrote:
> > 
> > Instead of using errno as a hidden argument to vfatal(), make it an 
> > _actual_ argument named 'code', ala the errc/warnc family, and rename it 
> > to vfatalc() to match the pattern set.
> > 
> > ok?
> > 
> > Philip Guenther
> > 
> 
> Isn't this code used elsewhere too?

benno is in the process of unifying all the log.c versions, as much
as possible.



Re: vmd: simplify fatal/fatalx errno handling

2016-10-16 Thread Mike Larkin
On Sun, Oct 16, 2016 at 02:55:39PM -0700, Philip Guenther wrote:
> 
> Instead of using errno as a hidden argument to vfatal(), make it an 
> _actual_ argument named 'code', ala the errc/warnc family, and rename it 
> to vfatalc() to match the pattern set.
> 
> ok?
> 
> Philip Guenther
> 

Isn't this code used elsewhere too?

Reyk should probably comment on this.

-ml

> Index: log.c
> ===
> RCS file: /data/src/openbsd/src/usr.sbin/vmd/log.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 log.c
> --- log.c 12 Oct 2016 11:47:34 -  1.3
> +++ log.c 16 Oct 2016 21:17:40 -
> @@ -165,11 +165,10 @@ log_debug(const char *emsg, ...)
>  }
>  
>  static void
> -vfatal(const char *emsg, va_list ap)
> +vfatalc(int code, const char *emsg, va_list ap)
>  {
>   static char s[BUFSIZ];
>   const char  *sep;
> - int  saved_errno = errno;
>  
>   if (emsg != NULL) {
>   (void)vsnprintf(s, sizeof(s), emsg, ap);
> @@ -178,9 +177,9 @@ vfatal(const char *emsg, va_list ap)
>   s[0] = '\0';
>   sep = "";
>   }
> - if (saved_errno)
> + if (code)
>   logit(LOG_CRIT, "%s: %s%s%s",
> - log_procname, s, sep, strerror(saved_errno));
> + log_procname, s, sep, strerror(code));
>   else
>   logit(LOG_CRIT, "%s%s%s", log_procname, sep, s);
>  }
> @@ -191,7 +190,7 @@ fatal(const char *emsg, ...)
>   va_list ap;
>  
>   va_start(ap, emsg);
> - vfatal(emsg, ap);
> + vfatalc(errno, emsg, ap);
>   va_end(ap);
>   exit(1);
>  }
> @@ -201,9 +200,8 @@ fatalx(const char *emsg, ...)
>  {
>   va_list ap;
>  
> - errno = 0;
>   va_start(ap, emsg);
> - vfatal(emsg, ap);
> + vfatalc(0, emsg, ap);
>   va_end(ap);
>   exit(1);
>  }
>