yyerror and ospfd reload

2013-03-03 Thread Stuart Henderson
If ospfd is running and you attempt to reload a configuration file
but it has an error (or the parser thinks it has an error even though
the file is valid..), yyerror just prints to stderr so the message
is lost unless you're running ospfd in the foreground.

I wrote a diff to handle this (included at the bottom of this mail)
but then I noticed that bgpd already does the same, so this just syncs
with bgpd's handling. OK?

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
retrieving revision 1.73
diff -u -p -r1.73 parse.y
--- parse.y 13 Dec 2010 13:43:37 -  1.73
+++ parse.y 3 Mar 2013 21:13:58 -
@@ -36,6 +36,7 @@
 #include stdarg.h
 #include stdio.h
 #include string.h
+#include syslog.h
 
 #include ospf.h
 #include ospfd.h
@@ -692,14 +693,16 @@ struct keywords {
 int
 yyerror(const char *fmt, ...)
 {
-   va_list ap;
+   va_list  ap;
+   char*nfmt;
 
file-errors++;
va_start(ap, fmt);
-   fprintf(stderr, %s:%d: , file-name, yylval.lineno);
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n);
+   if (asprintf(nfmt, %s:%d: %s, file-name, yylval.lineno, fmt) == -1)
+   fatalx(yyerror asprintf);
+   vlog(LOG_CRIT, nfmt, ap);
va_end(ap);
+   free(nfmt);
return (0);
 }
 


Here's the one I came up with myself FWIW.

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
retrieving revision 1.73
diff -u -p -r1.73 parse.y
--- parse.y 13 Dec 2010 13:43:37 -  1.73
+++ parse.y 3 Mar 2013 21:01:25 -
@@ -693,12 +693,14 @@ int
 yyerror(const char *fmt, ...)
 {
va_list ap;
+   char *buf;
 
file-errors++;
va_start(ap, fmt);
-   fprintf(stderr, %s:%d: , file-name, yylval.lineno);
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n);
+   if (vasprintf(buf, fmt, ap) = 0) {
+   log_warnx(%s:%d: %s, file-name, yylval.lineno, buf);
+   free(buf);
+   }
va_end(ap);
return (0);
 }



Re: yyerror and ospfd reload

2013-03-03 Thread Theo de Raadt
I like the bgpd behaviour, and all the similar daemons should have the
same behaviour.

 If ospfd is running and you attempt to reload a configuration file
 but it has an error (or the parser thinks it has an error even though
 the file is valid..), yyerror just prints to stderr so the message
 is lost unless you're running ospfd in the foreground.
 
 I wrote a diff to handle this (included at the bottom of this mail)
 but then I noticed that bgpd already does the same, so this just syncs
 with bgpd's handling. OK?
 
 Index: parse.y
 ===
 RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
 retrieving revision 1.73
 diff -u -p -r1.73 parse.y
 --- parse.y   13 Dec 2010 13:43:37 -  1.73
 +++ parse.y   3 Mar 2013 21:13:58 -
 @@ -36,6 +36,7 @@
  #include stdarg.h
  #include stdio.h
  #include string.h
 +#include syslog.h
  
  #include ospf.h
  #include ospfd.h
 @@ -692,14 +693,16 @@ struct keywords {
  int
  yyerror(const char *fmt, ...)
  {
 - va_list ap;
 + va_list  ap;
 + char*nfmt;
  
   file-errors++;
   va_start(ap, fmt);
 - fprintf(stderr, %s:%d: , file-name, yylval.lineno);
 - vfprintf(stderr, fmt, ap);
 - fprintf(stderr, \n);
 + if (asprintf(nfmt, %s:%d: %s, file-name, yylval.lineno, fmt) == -1)
 + fatalx(yyerror asprintf);
 + vlog(LOG_CRIT, nfmt, ap);
   va_end(ap);
 + free(nfmt);
   return (0);
  }
  
 
 
 Here's the one I came up with myself FWIW.
 
 Index: parse.y
 ===
 RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
 retrieving revision 1.73
 diff -u -p -r1.73 parse.y
 --- parse.y   13 Dec 2010 13:43:37 -  1.73
 +++ parse.y   3 Mar 2013 21:01:25 -
 @@ -693,12 +693,14 @@ int
  yyerror(const char *fmt, ...)
  {
   va_list ap;
 + char *buf;
  
   file-errors++;
   va_start(ap, fmt);
 - fprintf(stderr, %s:%d: , file-name, yylval.lineno);
 - vfprintf(stderr, fmt, ap);
 - fprintf(stderr, \n);
 + if (vasprintf(buf, fmt, ap) = 0) {
 + log_warnx(%s:%d: %s, file-name, yylval.lineno, buf);
 + free(buf);
 + }
   va_end(ap);
   return (0);
  }
 



Re: yyerror and ospfd reload

2013-03-03 Thread Stuart Henderson
On 2013/03/03 14:25, Theo de Raadt wrote:
 I like the bgpd behaviour, and all the similar daemons should have the
 same behaviour.

Here it is for all of the daemons which have parse.y and log.c; it only
really affects the ones which support reload, but I think it makes sense to
use the same code in all for consistency.

ldapd, bgpd, npppd, ntpd already use vlog, hostapd doesn't use log.c.

Index: ospf6d/parse.y
===
RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
retrieving revision 1.21
diff -u -p -r1.21 parse.y
--- ospf6d/parse.y  27 Jun 2011 03:07:26 -  1.21
+++ ospf6d/parse.y  3 Mar 2013 22:13:10 -
@@ -38,6 +38,7 @@
 #include stdarg.h
 #include stdio.h
 #include string.h
+#include syslog.h
 
 #include ospf6.h
 #include ospf6d.h
@@ -516,14 +517,16 @@ struct keywords {
 int
 yyerror(const char *fmt, ...)
 {
-   va_list ap;
+   va_list  ap;
+   char*nfmt;
 
file-errors++;
va_start(ap, fmt);
-   fprintf(stderr, %s:%d: , file-name, yylval.lineno);
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n);
+   if (asprintf(nfmt, %s:%d: %s, file-name, yylval.lineno, fmt) == -1)
+   fatalx(yyerror asprintf);
+   vlog(LOG_CRIT, nfmt, ap);
va_end(ap);
+   free(nfmt);
return (0);
 }
 
Index: ospfd/parse.y
===
RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
retrieving revision 1.73
diff -u -p -r1.73 parse.y
--- ospfd/parse.y   13 Dec 2010 13:43:37 -  1.73
+++ ospfd/parse.y   3 Mar 2013 22:13:10 -
@@ -36,6 +36,7 @@
 #include stdarg.h
 #include stdio.h
 #include string.h
+#include syslog.h
 
 #include ospf.h
 #include ospfd.h
@@ -692,14 +693,16 @@ struct keywords {
 int
 yyerror(const char *fmt, ...)
 {
-   va_list ap;
+   va_list  ap;
+   char*nfmt;
 
file-errors++;
va_start(ap, fmt);
-   fprintf(stderr, %s:%d: , file-name, yylval.lineno);
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n);
+   if (asprintf(nfmt, %s:%d: %s, file-name, yylval.lineno, fmt) == -1)
+   fatalx(yyerror asprintf);
+   vlog(LOG_CRIT, nfmt, ap);
va_end(ap);
+   free(nfmt);
return (0);
 }
 
Index: dvmrpd/parse.y
===
RCS file: /cvs/src/usr.sbin/dvmrpd/parse.y,v
retrieving revision 1.22
diff -u -p -r1.22 parse.y
--- dvmrpd/parse.y  31 Dec 2010 21:22:42 -  1.22
+++ dvmrpd/parse.y  3 Mar 2013 22:13:10 -
@@ -358,14 +358,16 @@ struct keywords {
 int
 yyerror(const char *fmt, ...)
 {
-   va_list ap;
+   va_list  ap;
+   char*nfmt;
 
file-errors++;
va_start(ap, fmt);
-   fprintf(stderr, %s:%d: , file-name, yylval.lineno);
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n);
+   if (asprintf(nfmt, %s:%d: %s, file-name, yylval.lineno, fmt) == -1)
+   fatalx(yyerror asprintf);
+   vlog(LOG_CRIT, nfmt, ap);
va_end(ap);
+   free(nfmt);
return (0);
 }
 
Index: ifstated/ifstated.h
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v
retrieving revision 1.6
diff -u -p -r1.6 ifstated.h
--- ifstated/ifstated.h 4 Feb 2010 13:50:14 -   1.6
+++ ifstated/ifstated.h 3 Mar 2013 22:14:38 -
@@ -142,5 +142,6 @@ voidlog_warn(const char *, ...);
 void   log_warnx(const char *, ...);
 void   log_info(const char *, ...);
 void   log_debug(const char *, ...);
+void   vlog(int, const char *, va_list);
 __dead void fatal(const char *);
 __dead void fatalx(const char *);
Index: ifstated/parse.y
===
RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v
retrieving revision 1.30
diff -u -p -r1.30 parse.y
--- ifstated/parse.y3 Aug 2010 18:42:40 -   1.30
+++ ifstated/parse.y3 Mar 2013 22:13:10 -
@@ -355,13 +355,15 @@ int
 yyerror(const char *fmt, ...)
 {
va_list  ap;
+   char*nfmt;
 
file-errors++;
va_start(ap, fmt);
-   fprintf(stderr, %s:%d: , file-name, yylval.lineno);
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n);
+   if (asprintf(nfmt, %s:%d: %s, file-name, yylval.lineno, fmt) == -1)
+   fatalx(yyerror asprintf);
+   vlog(LOG_CRIT, nfmt, ap);
va_end(ap);
+   free(nfmt);
return (0);
 }
 
Index: relayd/parse.y
===
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.168
diff -u -p -r1.168 parse.y
--- relayd/parse.y  19 Oct 2012 16:49:50 -  1.168
+++ relayd/parse.y  3 Mar 2013 22:13:10 -
@@ -51,6 +51,7 @@
 #include netdb.h
 #include string.h
 #include ifaddrs.h
+#include