Rob Pierce([email protected]) on 2017.06.11 18:04:31 -0400:
> This minimizes differences with the latest log.c.
>
> I was not sure how to handle verbosity, as the current implementation is
> verbose by default in debug mode. The diff below requires actually
> requesting (double) verbosity on the command line in order to retain the
> same behaviour (in debug mode).
Thanks.
can you redo this with the log.c from bgpd/ospfd/... ?
The difference is exactly in the handling of verbose, and that way we dont
change the -v semantics right now.
I havent unified the verbose handling yet, but if you diff them you'll see
its only a small difference now. My plan is to narrow all the log.c users
down to this small difference and then fix that in all of them in one go.
Also please add a log.h with the declarations just like in bgpd/ospfd/...
/Benno
>
> Rob
>
> Index: ifstated.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 ifstated.c
> --- ifstated.c 30 May 2013 19:22:48 -0000 1.41
> +++ ifstated.c 11 Jun 2017 21:51:16 -0000
> @@ -32,10 +32,11 @@
> #include <net/route.h>
> #include <netinet/in.h>
>
> +#include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> -#include <signal.h>
> +#include <syslog.h>
> #include <err.h>
> #include <event.h>
> #include <unistd.h>
> @@ -86,14 +87,12 @@ main(int argc, char *argv[])
> {
> struct timeval tv;
> int ch;
> - int debug = 0;
> -
> - log_init(1);
> + int debug = 0, verbose = 0;
>
> while ((ch = getopt(argc, argv, "dD:f:hniv")) != -1) {
> switch (ch) {
> case 'd':
> - debug = 1;
> + debug = 2;
> break;
> case 'D':
> if (cmdline_symset(optarg) < 0)
> @@ -113,6 +112,7 @@ main(int argc, char *argv[])
> opt_inhibit = 1;
> break;
> case 'v':
> + verbose++;
> if (opts & IFSD_OPT_VERBOSE)
> opts |= IFSD_OPT_VERBOSE2;
> opts |= IFSD_OPT_VERBOSE;
> @@ -122,6 +122,9 @@ main(int argc, char *argv[])
> }
> }
>
> + /* log to stderr until daemonized */
> + log_init(debug ? debug : 1, LOG_DAEMON);
> +
> argc -= optind;
> argv += optind;
> if (argc > 0)
> @@ -138,7 +141,8 @@ main(int argc, char *argv[])
> daemon(1, 0);
>
> event_init();
> - log_init(debug);
> + log_init(debug, LOG_DAEMON);
> + log_setverbose(verbose);
>
> signal_set(&sigchld_ev, SIGCHLD, sigchld_handler, NULL);
> signal_add(&sigchld_ev, NULL);
> @@ -169,12 +173,12 @@ startup_handler(int fd, short event, voi
> rtfilter = ROUTE_FILTER(RTM_IFINFO);
> if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER,
> &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */
> - log_warn("startup_handler: setsockopt msgfilter");
> + log_warn("%s: setsockopt msgfilter", __func__);
>
> rtfilter = RTABLE_ANY;
> if (setsockopt(rt_fd, PF_ROUTE, ROUTE_TABLEFILTER,
> &rtfilter, sizeof(rtfilter)) == -1) /* not fatal */
> - log_warn("startup_handler: setsockopt tablefilter");
> + log_warn("%s: setsockopt tablefilter", __func__);
>
> event_set(&rt_msg_ev, rt_fd, EV_READ|EV_PERSIST, rt_msg_handler, NULL);
> event_add(&rt_msg_ev, NULL);
> @@ -580,7 +584,7 @@ do_action(struct ifsd_action *action)
> }
> break;
> default:
> - log_debug("do_action: unknown action %d", action->type);
> + log_debug("%s: unknown action %d", __func__, action->type);
> break;
> }
> }
> Index: ifstated.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v
> retrieving revision 1.10
> diff -u -p -r1.10 ifstated.h
> --- ifstated.h 19 Jul 2016 08:04:53 -0000 1.10
> +++ ifstated.h 11 Jun 2017 21:51:16 -0000
> @@ -137,7 +137,10 @@ int cmdline_symset(char *);
> void clear_config(struct ifsd_config *);
>
> /* log.c */
> -void log_init(int);
> +void log_init(int, int);
> +void log_procinit(const char *);
> +void log_setverbose(int);
> +int log_getverbose(void);
> void log_warn(const char *, ...)
> __attribute__((__format__ (printf, 1, 2)));
> void log_warnx(const char *, ...)
> @@ -146,9 +149,11 @@ void log_info(const char *, ...)
> __attribute__((__format__ (printf, 1, 2)));
> void log_debug(const char *, ...)
> __attribute__((__format__ (printf, 1, 2)));
> -void vlog(int, const char *, va_list)
> - __attribute__((__format__ (printf, 2, 0)));
> void logit(int, const char *, ...)
> __attribute__((__format__ (printf, 2, 3)));
> -void fatal(const char *) __dead;
> -void fatalx(const char *) __dead;
> +void vlog(int, const char *, va_list)
> + __attribute__((__format__ (printf, 2, 0)));
> +__dead void fatal(const char *, ...)
> + __attribute__((__format__ (printf, 1, 2)));
> +__dead void fatalx(const char *, ...)
> + __attribute__((__format__ (printf, 1, 2)));
> Index: log.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ifstated/log.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 log.c
> --- log.c 21 Mar 2017 12:06:55 -0000 1.4
> +++ log.c 11 Jun 2017 21:51:16 -0000
> @@ -16,40 +16,74 @@
> * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> */
>
> -#include <errno.h>
> -#include <stdarg.h>
> #include <stdio.h>
> #include <stdlib.h>
> +#include <stdarg.h>
> #include <string.h>
> #include <syslog.h>
> +#include <errno.h>
> #include <time.h>
>
> -void log_init(int);
> -void log_warn(const char *, ...);
> -void log_warnx(const char *, ...);
> -void log_info(const char *, ...);
> -void log_debug(const char *, ...);
> -__dead void fatal(const char *);
> -__dead void fatalx(const char *);
> -void vlog(int, const char *, va_list);
> -void logit(int, const char *, ...);
> -
> -int debug;
> +static int debug;
> +static int verbose;
> +const char *log_procname;
> +
> +void log_init(int, int);
> +void log_procinit(const char *);
> +void log_setverbose(int);
> +int log_getverbose(void);
> +void log_warn(const char *, ...)
> + __attribute__((__format__ (printf, 1, 2)));
> +void log_warnx(const char *, ...)
> + __attribute__((__format__ (printf, 1, 2)));
> +void log_info(const char *, ...)
> + __attribute__((__format__ (printf, 1, 2)));
> +void log_debug(const char *, ...)
> + __attribute__((__format__ (printf, 1, 2)));
> +void logit(int, const char *, ...)
> + __attribute__((__format__ (printf, 2, 3)));
> +void vlog(int, const char *, va_list)
> + __attribute__((__format__ (printf, 2, 0)));
> +__dead void fatal(const char *, ...)
> + __attribute__((__format__ (printf, 1, 2)));
> +__dead void fatalx(const char *, ...)
> + __attribute__((__format__ (printf, 1, 2)));
>
> void
> -log_init(int n_debug)
> +log_init(int n_debug, int facility)
> {
> extern char *__progname;
>
> debug = n_debug;
> + verbose = n_debug;
> + log_procinit(__progname);
>
> if (!debug)
> - openlog(__progname, LOG_PID | LOG_NDELAY, LOG_DAEMON);
> + openlog(__progname, LOG_PID | LOG_NDELAY, facility);
>
> tzset();
> }
>
> void
> +log_procinit(const char *procname)
> +{
> + if (procname != NULL)
> + log_procname = procname;
> +}
> +
> +void
> +log_setverbose(int v)
> +{
> + verbose = v;
> +}
> +
> +int
> +log_getverbose(void)
> +{
> + return (verbose);
> +}
> +
> +void
> logit(int pri, const char *fmt, ...)
> {
> va_list ap;
> @@ -63,6 +97,7 @@ void
> vlog(int pri, const char *fmt, va_list ap)
> {
> char *nfmt;
> + int saved_errno = errno;
>
> if (debug) {
> /* best effort in out of mem situations */
> @@ -76,30 +111,36 @@ vlog(int pri, const char *fmt, va_list a
> fflush(stderr);
> } else
> vsyslog(pri, fmt, ap);
> +
> + errno = saved_errno;
> }
>
> void
> log_warn(const char *emsg, ...)
> {
> - char *nfmt;
> - va_list ap;
> + char *nfmt;
> + va_list ap;
> + int saved_errno = errno;
>
> /* best effort to even work in out of memory situations */
> if (emsg == NULL)
> - logit(LOG_ERR, "%s", strerror(errno));
> + logit(LOG_ERR, "%s", strerror(saved_errno));
> else {
> va_start(ap, emsg);
>
> - if (asprintf(&nfmt, "%s: %s", emsg, strerror(errno)) == -1) {
> + if (asprintf(&nfmt, "%s: %s", emsg,
> + strerror(saved_errno)) == -1) {
> /* we tried it... */
> vlog(LOG_ERR, emsg, ap);
> - logit(LOG_ERR, "%s", strerror(errno));
> + logit(LOG_ERR, "%s", strerror(saved_errno));
> } else {
> vlog(LOG_ERR, nfmt, ap);
> free(nfmt);
> }
> va_end(ap);
> }
> +
> + errno = saved_errno;
> }
>
> void
> @@ -127,31 +168,51 @@ log_debug(const char *emsg, ...)
> {
> va_list ap;
>
> - if (debug) {
> + if (verbose > 1) {
> va_start(ap, emsg);
> vlog(LOG_DEBUG, emsg, ap);
> va_end(ap);
> }
> }
>
> -void
> -fatal(const char *emsg)
> +static void
> +vfatalc(int code, const char *emsg, va_list ap)
> {
> - if (emsg == NULL)
> - logit(LOG_CRIT, "fatal: %s", strerror(errno));
> + static char s[BUFSIZ];
> + const char *sep;
> +
> + if (emsg != NULL) {
> + (void)vsnprintf(s, sizeof(s), emsg, ap);
> + sep = ": ";
> + } else {
> + s[0] = '\0';
> + sep = "";
> + }
> + if (code)
> + logit(LOG_CRIT, "%s: %s%s%s",
> + log_procname, s, sep, strerror(code));
> else
> - if (errno)
> - logit(LOG_CRIT, "fatal: %s: %s",
> - emsg, strerror(errno));
> - else
> - logit(LOG_CRIT, "fatal: %s", emsg);
> + logit(LOG_CRIT, "%s%s%s", log_procname, sep, s);
> +}
> +
> +void
> +fatal(const char *emsg, ...)
> +{
> + va_list ap;
>
> + va_start(ap, emsg);
> + vfatalc(errno, emsg, ap);
> + va_end(ap);
> exit(1);
> }
>
> void
> -fatalx(const char *emsg)
> +fatalx(const char *emsg, ...)
> {
> - errno = 0;
> - fatal(emsg);
> + va_list ap;
> +
> + va_start(ap, emsg);
> + vfatalc(0, emsg, ap);
> + va_end(ap);
> + exit(1);
> }
>
