Re: ifstated: consistent use of log.c
On Mon, Aug 07 2017, Rob Pierce wrote: > On Sun, Aug 06, 2017 at 06:47:38PM +0200, Jeremie Courreges-Anglas wrote: >> On Thu, Aug 03 2017, Rob Pierce wrote: >> > As a result ifstated.c no longer needs err.h. >> > >> > Index: ifstated.c >> > === >> > RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v >> > retrieving revision 1.56 >> > diff -u -p -r1.56 ifstated.c >> > --- ifstated.c 24 Jul 2017 12:33:59 - 1.56 >> > +++ ifstated.c 3 Aug 2017 23:59:13 - >> > @@ -37,7 +37,6 @@ >> > #include >> > #include >> > #include >> > -#include >> > #include >> > #include >> > #include >> > @@ -102,7 +101,7 @@ main(int argc, char *argv[]) >> >break; >> >case 'D': >> >if (cmdline_symset(optarg) < 0) >> > - errx(1, "could not parse macro definition %s", >> > + fatalx("could not parse macro definition %s", >> >optarg); >> >break; >> >case 'f': >> > @@ -135,7 +134,7 @@ main(int argc, char *argv[]) >> >if (opts & IFSD_OPT_NOACTION) { >> >if ((newconf = parse_config(configfile, opts)) == NULL) >> >exit(1); >> > - warnx("configuration OK"); >> > + fprintf(stderr, "configuration OK\n"); >> >> This changes the output from >> >> ifstated: configuration OK >> >> to >> >> configuration OK >> >> which is a bit less helpful. To keep the output the same, you could use >> warnx, fprintf + __progname/getprogname instead, or just >> >> errx(0, "configuration OK"); >> >> I don't really have a preference here. > > Good catch. I honestly didn't notice the change in output that was introduced > by my diff. I was following the code in other networking daemons. > > I would like to standardize on log.c and remove the err.h include, so maybe > the __progname approach is best if we want to keep the same output, or maybe > having a few lingering uses of err.h is ok. > > Many network daemons (below) do not reference themselves when confirming the > validity of their configuration file with the -n option. Is it better for > ifstated to retain it's historical output on the configuration check, or to be > made more consistent with other network daemons? Hah, thanks for checking the other daemons. I'm fine with your first diff then, ok jca@ > Regards, > > Rob > > bgpd/bgpd.c: fprintf(stderr, "configuration OK\n"); > dvmrpd/dvmrpd.c: fprintf(stderr, "configuration OK\n"); > eigrpd/eigrpd.c: fprintf(stderr, "configuration OK\n"); > httpd/httpd.c:fprintf(stderr, "configuration OK\n"); > ldapd/ldapd.c:fprintf(stderr, "configuration ok\n"); > ldpd/ldpd.c: fprintf(stderr, "configuration OK\n"); > ntpd/ntpd.c: fprintf(stderr, "configuration OK\n"); > ospf6d/ospf6d.c: fprintf(stderr, "configuration OK\n"); > ospfd/ospfd.c:fprintf(stderr, "configuration OK\n"); > radiusd/radiusd.c:fprintf(stderr, "configuration OK\n"); > relayd/relayd.c: fprintf(stderr, "configuration OK\n"); > ripd/ripd.c: fprintf(stderr, "configuration OK\n"); > sasyncd/sasyncd.c:fprintf(stderr, "configuration OK\n"); > smtpd/smtpd.c:fprintf(stderr, "configuration OK\n"); > snmpd/snmpd.c:fprintf(stderr, "configuration ok\n"); > switchd/switchd.c:fprintf(stderr, "configuration OK\n"); > vmd/vmd.c:fprintf(stderr, "configuration OK\n"); > ypldap/ypldap.c: fprintf(stderr, "configuration OK\n"); > >> >exit(0); >> >} >> > >> > @@ -147,7 +146,7 @@ main(int argc, char *argv[]) >> >log_setverbose(opts & IFSD_OPT_VERBOSE); >> > >> >if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) >> > - err(1, "no routing socket"); >> > + fatal("no routing socket"); >> > >> >rtfilter = ROUTE_FILTER(RTM_IFINFO); >> >if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, >> > @@ -604,7 +603,7 @@ fetch_ifstate(void) >> >struct ifaddrs *ifap, *ifa; >> > >> >if (getifaddrs(&ifap) != 0) >> > - err(1, "getifaddrs"); >> > + fatal("getifaddrs"); >> > >> >for (ifa = ifap; ifa; ifa = ifa->ifa_next) { >> >if (ifa->ifa_addr->sa_family == AF_LINK) { >> > >> >> >> -- >> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE > -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: ifstated: consistent use of log.c
On Sun, Aug 06, 2017 at 06:47:38PM +0200, Jeremie Courreges-Anglas wrote: > On Thu, Aug 03 2017, Rob Pierce wrote: > > As a result ifstated.c no longer needs err.h. > > > > Index: ifstated.c > > === > > RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v > > retrieving revision 1.56 > > diff -u -p -r1.56 ifstated.c > > --- ifstated.c 24 Jul 2017 12:33:59 - 1.56 > > +++ ifstated.c 3 Aug 2017 23:59:13 - > > @@ -37,7 +37,6 @@ > > #include > > #include > > #include > > -#include > > #include > > #include > > #include > > @@ -102,7 +101,7 @@ main(int argc, char *argv[]) > > break; > > case 'D': > > if (cmdline_symset(optarg) < 0) > > - errx(1, "could not parse macro definition %s", > > + fatalx("could not parse macro definition %s", > > optarg); > > break; > > case 'f': > > @@ -135,7 +134,7 @@ main(int argc, char *argv[]) > > if (opts & IFSD_OPT_NOACTION) { > > if ((newconf = parse_config(configfile, opts)) == NULL) > > exit(1); > > - warnx("configuration OK"); > > + fprintf(stderr, "configuration OK\n"); > > This changes the output from > > ifstated: configuration OK > > to > > configuration OK > > which is a bit less helpful. To keep the output the same, you could use > warnx, fprintf + __progname/getprogname instead, or just > > errx(0, "configuration OK"); > > I don't really have a preference here. Good catch. I honestly didn't notice the change in output that was introduced by my diff. I was following the code in other networking daemons. I would like to standardize on log.c and remove the err.h include, so maybe the __progname approach is best if we want to keep the same output, or maybe having a few lingering uses of err.h is ok. Many network daemons (below) do not reference themselves when confirming the validity of their configuration file with the -n option. Is it better for ifstated to retain it's historical output on the configuration check, or to be made more consistent with other network daemons? Regards, Rob bgpd/bgpd.c:fprintf(stderr, "configuration OK\n"); dvmrpd/dvmrpd.c:fprintf(stderr, "configuration OK\n"); eigrpd/eigrpd.c:fprintf(stderr, "configuration OK\n"); httpd/httpd.c: fprintf(stderr, "configuration OK\n"); ldapd/ldapd.c: fprintf(stderr, "configuration ok\n"); ldpd/ldpd.c:fprintf(stderr, "configuration OK\n"); ntpd/ntpd.c:fprintf(stderr, "configuration OK\n"); ospf6d/ospf6d.c:fprintf(stderr, "configuration OK\n"); ospfd/ospfd.c: fprintf(stderr, "configuration OK\n"); radiusd/radiusd.c: fprintf(stderr, "configuration OK\n"); relayd/relayd.c:fprintf(stderr, "configuration OK\n"); ripd/ripd.c:fprintf(stderr, "configuration OK\n"); sasyncd/sasyncd.c: fprintf(stderr, "configuration OK\n"); smtpd/smtpd.c: fprintf(stderr, "configuration OK\n"); snmpd/snmpd.c: fprintf(stderr, "configuration ok\n"); switchd/switchd.c: fprintf(stderr, "configuration OK\n"); vmd/vmd.c: fprintf(stderr, "configuration OK\n"); ypldap/ypldap.c:fprintf(stderr, "configuration OK\n"); > > exit(0); > > } > > > > @@ -147,7 +146,7 @@ main(int argc, char *argv[]) > > log_setverbose(opts & IFSD_OPT_VERBOSE); > > > > if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) > > - err(1, "no routing socket"); > > + fatal("no routing socket"); > > > > rtfilter = ROUTE_FILTER(RTM_IFINFO); > > if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, > > @@ -604,7 +603,7 @@ fetch_ifstate(void) > > struct ifaddrs *ifap, *ifa; > > > > if (getifaddrs(&ifap) != 0) > > - err(1, "getifaddrs"); > > + fatal("getifaddrs"); > > > > for (ifa = ifap; ifa; ifa = ifa->ifa_next) { > > if (ifa->ifa_addr->sa_family == AF_LINK) { > > > > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: ifstated: consistent use of log.c
On Thu, Aug 03 2017, Rob Pierce wrote: > As a result ifstated.c no longer needs err.h. > > Index: ifstated.c > === > RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v > retrieving revision 1.56 > diff -u -p -r1.56 ifstated.c > --- ifstated.c24 Jul 2017 12:33:59 - 1.56 > +++ ifstated.c3 Aug 2017 23:59:13 - > @@ -37,7 +37,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -102,7 +101,7 @@ main(int argc, char *argv[]) > break; > case 'D': > if (cmdline_symset(optarg) < 0) > - errx(1, "could not parse macro definition %s", > + fatalx("could not parse macro definition %s", > optarg); > break; > case 'f': > @@ -135,7 +134,7 @@ main(int argc, char *argv[]) > if (opts & IFSD_OPT_NOACTION) { > if ((newconf = parse_config(configfile, opts)) == NULL) > exit(1); > - warnx("configuration OK"); > + fprintf(stderr, "configuration OK\n"); This changes the output from ifstated: configuration OK to configuration OK which is a bit less helpful. To keep the output the same, you could use warnx, fprintf + __progname/getprogname instead, or just errx(0, "configuration OK"); I don't really have a preference here. > exit(0); > } > > @@ -147,7 +146,7 @@ main(int argc, char *argv[]) > log_setverbose(opts & IFSD_OPT_VERBOSE); > > if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) > - err(1, "no routing socket"); > + fatal("no routing socket"); > > rtfilter = ROUTE_FILTER(RTM_IFINFO); > if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, > @@ -604,7 +603,7 @@ fetch_ifstate(void) > struct ifaddrs *ifap, *ifa; > > if (getifaddrs(&ifap) != 0) > - err(1, "getifaddrs"); > + fatal("getifaddrs"); > > for (ifa = ifap; ifa; ifa = ifa->ifa_next) { > if (ifa->ifa_addr->sa_family == AF_LINK) { > -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
ifstated: consistent use of log.c
As a result ifstated.c no longer needs err.h. Index: ifstated.c === RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v retrieving revision 1.56 diff -u -p -r1.56 ifstated.c --- ifstated.c 24 Jul 2017 12:33:59 - 1.56 +++ ifstated.c 3 Aug 2017 23:59:13 - @@ -37,7 +37,6 @@ #include #include #include -#include #include #include #include @@ -102,7 +101,7 @@ main(int argc, char *argv[]) break; case 'D': if (cmdline_symset(optarg) < 0) - errx(1, "could not parse macro definition %s", + fatalx("could not parse macro definition %s", optarg); break; case 'f': @@ -135,7 +134,7 @@ main(int argc, char *argv[]) if (opts & IFSD_OPT_NOACTION) { if ((newconf = parse_config(configfile, opts)) == NULL) exit(1); - warnx("configuration OK"); + fprintf(stderr, "configuration OK\n"); exit(0); } @@ -147,7 +146,7 @@ main(int argc, char *argv[]) log_setverbose(opts & IFSD_OPT_VERBOSE); if ((rt_fd = socket(PF_ROUTE, SOCK_RAW, 0)) < 0) - err(1, "no routing socket"); + fatal("no routing socket"); rtfilter = ROUTE_FILTER(RTM_IFINFO); if (setsockopt(rt_fd, PF_ROUTE, ROUTE_MSGFILTER, @@ -604,7 +603,7 @@ fetch_ifstate(void) struct ifaddrs *ifap, *ifa; if (getifaddrs(&ifap) != 0) - err(1, "getifaddrs"); + fatal("getifaddrs"); for (ifa = ifap; ifa; ifa = ifa->ifa_next) { if (ifa->ifa_addr->sa_family == AF_LINK) {