On Fri, Oct 13, 2023 at 07:16:13PM +0200, Theo Buehler wrote:
> On Fri, Oct 13, 2023 at 07:01:06PM +0200, Claudio Jeker wrote:
> > Extending the format string with the peer info is a bad idea.
> > The reason is DNS^WIPv6 and scoped addresses which add a % to the
> > string returned by log_fmt_peer.
> >
> > So instead vasprintf() the emsg and then just use logit().
>
> Ugh. That's nasty.
>
> Diff reads fine. There's a change of behavior in that logit() does
> asprintf() but will not fatal on asprintf() failure.
In a way that was already the case before. vlog() is doing this asprintf
dance. Which is dumb.
I'm tempted to rewrite more of this and just use buffered stdio and
fflush() to get the atomic behaviour.
> ok tb
>
> > --
> > :wq Claudio
> >
> > Index: logmsg.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/logmsg.c,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 logmsg.c
> > --- logmsg.c 24 Aug 2022 17:14:02 -0000 1.9
> > +++ logmsg.c 13 Oct 2023 16:43:55 -0000
> > @@ -60,55 +60,54 @@ log_fmt_peer(const struct peer_config *p
> > void
> > log_peer_info(const struct peer_config *peer, const char *emsg, ...)
> > {
> > - char *p, *nfmt;
> > + char *p, *msg;
> > va_list ap;
> >
> > p = log_fmt_peer(peer);
> > - if (asprintf(&nfmt, "%s: %s", p, emsg) == -1)
> > - fatal(NULL);
> > va_start(ap, emsg);
> > - vlog(LOG_INFO, nfmt, ap);
> > + if (vasprintf(&msg, emsg, ap) == -1)
> > + fatal(NULL);
> > va_end(ap);
> > + logit(LOG_INFO, "%s: %s", p, msg);
> > + free(msg);
> > free(p);
> > - free(nfmt);
> > }
> >
> > void
> > log_peer_warn(const struct peer_config *peer, const char *emsg, ...)
> > {
> > - char *p, *nfmt;
> > + char *p, *msg;
> > va_list ap;
> > + int saved_errno = errno;
> >
> > p = log_fmt_peer(peer);
> > if (emsg == NULL) {
> > - if (asprintf(&nfmt, "%s: %s", p, strerror(errno)) == -1)
> > - fatal(NULL);
> > + logit(LOG_ERR, "%s: %s", p, strerror(saved_errno));
> > } else {
> > - if (asprintf(&nfmt, "%s: %s: %s", p, emsg, strerror(errno)) ==
> > - -1)
> > + va_start(ap, emsg);
> > + if (vasprintf(&msg, emsg, ap) == -1)
> > fatal(NULL);
> > + va_end(ap);
> > + logit(LOG_ERR, "%s: %s: %s", p, msg, strerror(saved_errno));
> > + free(msg);
> > }
> > - va_start(ap, emsg);
> > - vlog(LOG_ERR, nfmt, ap);
> > - va_end(ap);
> > free(p);
> > - free(nfmt);
> > }
> >
> > void
> > log_peer_warnx(const struct peer_config *peer, const char *emsg, ...)
> > {
> > - char *p, *nfmt;
> > + char *p, *msg;
> > va_list ap;
> >
> > p = log_fmt_peer(peer);
> > - if (asprintf(&nfmt, "%s: %s", p, emsg) == -1)
> > - fatal(NULL);
> > va_start(ap, emsg);
> > - vlog(LOG_ERR, nfmt, ap);
> > + if (vasprintf(&msg, emsg, ap) == -1)
> > + fatal(NULL);
> > va_end(ap);
> > + logit(LOG_ERR, "%s: %s", p, msg);
> > + free(msg);
> > free(p);
> > - free(nfmt);
> > }
> >
> > void
> >
>
--
:wq Claudio
