Re: m4(1): add -e flag support
On 6/14/2017 10:18 AM, marc espie wrote: > on wed, jun 14, 2017 at 09:58:54am -0400, brian callahan wrote: >> hi marc -- >> >> how's this version? >> also includes a slight tweak to the single -e flag regress test. >> >> ~brian > comment nit, otherwise good. > >> +extern int error_warns; /* make warnings cause exit_code > 0 */ > exit_code = 1, why > 0 when you actually specify the result everywhere ? Hmm... it's just supposed to be non-zero. Guess that morphed into > 0 in my head. No reason to not just specify it since that's what the code does.
Re: m4(1): add -e flag support
on wed, jun 14, 2017 at 09:58:54am -0400, brian callahan wrote: > hi marc -- > > how's this version? > also includes a slight tweak to the single -e flag regress test. > > ~brian comment nit, otherwise good. > +extern int error_warns; /* make warnings cause exit_code > 0 */ exit_code = 1, why > 0 when you actually specify the result everywhere ?
Re: m4(1): add -E flag support
Hi Marc -- How's this version? Also includes a slight tweak to the single -E flag regress test. ~Brian Index: usr.bin/m4/eval.c === RCS file: /cvs/src/usr.bin/m4/eval.c,v retrieving revision 1.74 diff -u -p -u -p -r1.74 eval.c --- usr.bin/m4/eval.c 5 Feb 2015 12:59:57 - 1.74 +++ usr.bin/m4/eval.c 14 Jun 2017 13:50:07 - @@ -269,6 +269,10 @@ expand_builtin(const char *argv[], int a warn("%s at line %lu: include(%s)", CURRENT_NAME, CURRENT_LINE, argv[2]); exit_code = 1; + if (fatal_warns) { + killdiv(); + exit(exit_code); + } } else err(1, "%s at line %lu: include(%s)", CURRENT_NAME, CURRENT_LINE, argv[2]); Index: usr.bin/m4/extern.h === RCS file: /cvs/src/usr.bin/m4/extern.h,v retrieving revision 1.54 diff -u -p -u -p -r1.54 extern.h --- usr.bin/m4/extern.h 12 May 2014 19:11:19 - 1.54 +++ usr.bin/m4/extern.h 14 Jun 2017 13:50:07 - @@ -58,6 +58,8 @@ extern void doesyscmd(const char *); extern void getdivfile(const char *); extern void doformat(const char *[], int); +extern void m4_warnx(const char *, ...); + /* look.c */ #define FLAG_UNTRACED 0 @@ -175,4 +177,6 @@ extern int synch_lines;/* line synchro extern int mimic_gnu; /* behaves like gnu-m4 */ extern int prefix_builtins;/* prefix builtin macros with m4_ */ +extern int error_warns;/* make warnings cause exit_code > 0 */ +extern int fatal_warns;/* make warnings fatal */ Index: usr.bin/m4/gnum4.c === RCS file: /cvs/src/usr.bin/m4/gnum4.c,v retrieving revision 1.50 diff -u -p -u -p -r1.50 gnum4.c --- usr.bin/m4/gnum4.c 29 Apr 2015 00:13:26 - 1.50 +++ usr.bin/m4/gnum4.c 14 Jun 2017 13:50:07 - @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -234,7 +235,7 @@ addchar(int c) } static char * -getstring() +getstring(void) { addchar('\0'); current = 0; @@ -255,11 +256,29 @@ exit_regerror(int er, regex_t *re, const m4errx(1, "regular expression error in %s: %s.", source, errbuf); } +/* warnx() plus check to see if we need to change exit code or exit. + * -E flag functionality. + */ +void +m4_warnx(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + vwarnx(fmt, ap); + va_end(ap); + + if (fatal_warns) + exit(1); + if (error_warns) + exit_code = 1; +} + static void add_sub(int n, const char *string, regex_t *re, regmatch_t *pm) { if (n > re->re_nsub) - warnx("No subexpression %d", n); + m4_warnx("No subexpression %d", n); /* Subexpressions that did not match are * not an error. */ else if (pm[n].rm_so != -1 && @@ -442,7 +461,7 @@ void dopatsubst(const char *argv[], int argc) { if (argc <= 3) { - warnx("Too few arguments to patsubst"); + m4_warnx("Too few arguments to patsubst"); return; } /* special case: empty regexp */ @@ -494,7 +513,7 @@ doregexp(const char *argv[], int argc) const char *source; if (argc <= 3) { - warnx("Too few arguments to regexp"); + m4_warnx("Too few arguments to regexp"); return; } /* special gnu case */ Index: usr.bin/m4/m4.1 === RCS file: /cvs/src/usr.bin/m4/m4.1,v retrieving revision 1.63 diff -u -p -u -p -r1.63 m4.1 --- usr.bin/m4/m4.1 14 Sep 2015 20:06:58 - 1.63 +++ usr.bin/m4/m4.1 14 Jun 2017 13:50:07 - @@ -38,7 +38,7 @@ .Nd macro language processor .Sh SYNOPSIS .Nm -.Op Fl gPs +.Op Fl EgPs .Oo .Sm off .Fl D Ar name Op No = Ar value @@ -127,6 +127,19 @@ turn on all options. .Pp By default, trace is set to .Qq eq . +.It Fl E +Set warnings to be fatal. +When a single +.Fl E +flag is specified, if warnings are issued, execution continues but +.Nm +will exit with a non-zero exit status. +When multiple +.Fl E +flags are specified, execution will halt upon issuing the first warning and +.Nm +will exit with a non-zero exit status. +This behaviour matches GNU-m4 1.4.9 and later. .It Fl g Activate GNU-m4 compatibility mode. In this mode, translit handles simple character @@ -434,7 +447,9 @@ Returns the current file's name. .Pp But note that the .Ic m4exit -macro can modify the exit st
Re: m4(1): add -E flag support
On Tue, Jun 13, 2017 at 05:05:56PM -0400, Brian Callahan wrote: > Hi -- > > Whoops, that was unintentional. Fixed. > > ~Brian Sorry for not looking sooner. I would rather you use two variables for -E flags. e.g., set error_warns for one -E flag, and fatal_warns for multiple -E flags. That way, most of your comments would be no longer necessary at all. ;) I mean: > Index: usr.bin/m4/eval.c > === > RCS file: /cvs/src/usr.bin/m4/eval.c,v > retrieving revision 1.74 > diff -u -p -u -p -r1.74 eval.c > --- usr.bin/m4/eval.c 5 Feb 2015 12:59:57 - 1.74 > +++ usr.bin/m4/eval.c 13 Jun 2017 21:03:15 - > @@ -269,6 +269,12 @@ expand_builtin(const char *argv[], int a > warn("%s at line %lu: include(%s)", > CURRENT_NAME, CURRENT_LINE, > argv[2]); > exit_code = 1; That comment: > + /* exit immediately if multiple -E flags > + */ > + if (fatal_warns == 2) { > + killdiv(); > + exit(exit_code); > + } > } else > err(1, "%s at line %lu: include(%s)", > CURRENT_NAME, CURRENT_LINE, > argv[2]); > Index: usr.bin/m4/gnum4.c > === > RCS file: /cvs/src/usr.bin/m4/gnum4.c,v > retrieving revision 1.50 > diff -u -p -u -p -r1.50 gnum4.c > --- usr.bin/m4/gnum4.c29 Apr 2015 00:13:26 - 1.50 > +++ usr.bin/m4/gnum4.c13 Jun 2017 21:03:15 - > @@ -255,11 +256,35 @@ exit_regerror(int er, regex_t *re, const > m4errx(1, "regular expression error in %s: %s.", source, errbuf); > } > > +/* warnx() plus check to see if we need to change exit code or exit. > + * -E flag functionality. > + */ > +void > +m4_warnx(const char *fmt, ...) > +{ > + va_list ap; > + > + va_start(ap, fmt); > + vwarnx(fmt, ap); > + va_end(ap); > + and that code: > + /* Do nothing if no -E flags, set exit_code > 0 but keep going > + * if one -E flag, exit immediately with exit status > 0 if > + * two or more -E flags. > + */ > + if (fatal_warns == 0) > + return; > + else if (fatal_warns == 1) > + exit_code = 1; > + else > + exit(1); > +} > + which would become something like: if (fatal_warns) exit(1); if (error_warns) exit_code = 1; no need for any comment
Re: m4(1): add -E flag support
On Tue, 13 Jun 2017 17:05:56 -0400, Brian Callahan wrote: > Whoops, that was unintentional. Fixed. Looks good to me. - todd
Re: m4(1): add -E flag support
Hi -- Whoops, that was unintentional. Fixed. ~Brian Index: usr.bin/m4/eval.c === RCS file: /cvs/src/usr.bin/m4/eval.c,v retrieving revision 1.74 diff -u -p -u -p -r1.74 eval.c --- usr.bin/m4/eval.c 5 Feb 2015 12:59:57 - 1.74 +++ usr.bin/m4/eval.c 13 Jun 2017 21:03:15 - @@ -269,6 +269,12 @@ expand_builtin(const char *argv[], int a warn("%s at line %lu: include(%s)", CURRENT_NAME, CURRENT_LINE, argv[2]); exit_code = 1; + /* exit immediately if multiple -E flags +*/ + if (fatal_warns == 2) { + killdiv(); + exit(exit_code); + } } else err(1, "%s at line %lu: include(%s)", CURRENT_NAME, CURRENT_LINE, argv[2]); Index: usr.bin/m4/extern.h === RCS file: /cvs/src/usr.bin/m4/extern.h,v retrieving revision 1.54 diff -u -p -u -p -r1.54 extern.h --- usr.bin/m4/extern.h 12 May 2014 19:11:19 - 1.54 +++ usr.bin/m4/extern.h 13 Jun 2017 21:03:15 - @@ -58,6 +58,8 @@ extern void doesyscmd(const char *); extern void getdivfile(const char *); extern void doformat(const char *[], int); +extern void m4_warnx(const char *, ...); + /* look.c */ #define FLAG_UNTRACED 0 @@ -175,4 +177,5 @@ extern int synch_lines;/* line synchro extern int mimic_gnu; /* behaves like gnu-m4 */ extern int prefix_builtins;/* prefix builtin macros with m4_ */ +extern int fatal_warns;/* make warnings fatal */ Index: usr.bin/m4/gnum4.c === RCS file: /cvs/src/usr.bin/m4/gnum4.c,v retrieving revision 1.50 diff -u -p -u -p -r1.50 gnum4.c --- usr.bin/m4/gnum4.c 29 Apr 2015 00:13:26 - 1.50 +++ usr.bin/m4/gnum4.c 13 Jun 2017 21:03:15 - @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -234,7 +235,7 @@ addchar(int c) } static char * -getstring() +getstring(void) { addchar('\0'); current = 0; @@ -255,11 +256,35 @@ exit_regerror(int er, regex_t *re, const m4errx(1, "regular expression error in %s: %s.", source, errbuf); } +/* warnx() plus check to see if we need to change exit code or exit. + * -E flag functionality. + */ +void +m4_warnx(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + vwarnx(fmt, ap); + va_end(ap); + + /* Do nothing if no -E flags, set exit_code > 0 but keep going +* if one -E flag, exit immediately with exit status > 0 if +* two or more -E flags. +*/ + if (fatal_warns == 0) + return; + else if (fatal_warns == 1) + exit_code = 1; + else + exit(1); +} + static void add_sub(int n, const char *string, regex_t *re, regmatch_t *pm) { if (n > re->re_nsub) - warnx("No subexpression %d", n); + m4_warnx("No subexpression %d", n); /* Subexpressions that did not match are * not an error. */ else if (pm[n].rm_so != -1 && @@ -442,7 +467,7 @@ void dopatsubst(const char *argv[], int argc) { if (argc <= 3) { - warnx("Too few arguments to patsubst"); + m4_warnx("Too few arguments to patsubst"); return; } /* special case: empty regexp */ @@ -494,7 +519,7 @@ doregexp(const char *argv[], int argc) const char *source; if (argc <= 3) { - warnx("Too few arguments to regexp"); + m4_warnx("Too few arguments to regexp"); return; } /* special gnu case */ Index: usr.bin/m4/m4.1 === RCS file: /cvs/src/usr.bin/m4/m4.1,v retrieving revision 1.63 diff -u -p -u -p -r1.63 m4.1 --- usr.bin/m4/m4.1 14 Sep 2015 20:06:58 - 1.63 +++ usr.bin/m4/m4.1 13 Jun 2017 21:03:15 - @@ -38,7 +38,7 @@ .Nd macro language processor .Sh SYNOPSIS .Nm -.Op Fl gPs +.Op Fl EgPs .Oo .Sm off .Fl D Ar name Op No = Ar value @@ -127,6 +127,19 @@ turn on all options. .Pp By default, trace is set to .Qq eq . +.It Fl E +Set warnings to be fatal. +When a single +.Fl E +flag is specified, if warnings are issued, execution continues but +.Nm +will exit with a non-zero exit status. +When multiple +.Fl E +flags are specified, execution will halt upon issuing the first warning and +.Nm +will exit with a non-zero exit status. +This behaviour matches GNU
Re: m4(1): add -E flag support
On Tue, 13 Jun 2017 16:31:20 -0400, Brian Callahan wrote: > Updated diff attached. Wrapped everything up in an m4_warnx() call > and added a regress test for single -E. Thanks! In tokenizer.l you don't need the "m4: " prefix in the warning messages or the trailing newline, both will be added by vwarnx(). - todd
Re: m4(1): add -E flag support
Hi -- Updated diff attached. Wrapped everything up in an m4_warnx() call and added a regress test for single -E. Thanks! The one fatal_warns check in eval.c seems like a special case, so I left that one as it was. ~Brian Index: usr.bin/m4/eval.c === RCS file: /cvs/src/usr.bin/m4/eval.c,v retrieving revision 1.74 diff -u -p -u -p -r1.74 eval.c --- usr.bin/m4/eval.c 5 Feb 2015 12:59:57 - 1.74 +++ usr.bin/m4/eval.c 13 Jun 2017 20:22:09 - @@ -269,6 +269,12 @@ expand_builtin(const char *argv[], int a warn("%s at line %lu: include(%s)", CURRENT_NAME, CURRENT_LINE, argv[2]); exit_code = 1; + /* exit immediately if multiple -E flags +*/ + if (fatal_warns == 2) { + killdiv(); + exit(exit_code); + } } else err(1, "%s at line %lu: include(%s)", CURRENT_NAME, CURRENT_LINE, argv[2]); Index: usr.bin/m4/extern.h === RCS file: /cvs/src/usr.bin/m4/extern.h,v retrieving revision 1.54 diff -u -p -u -p -r1.54 extern.h --- usr.bin/m4/extern.h 12 May 2014 19:11:19 - 1.54 +++ usr.bin/m4/extern.h 13 Jun 2017 20:22:09 - @@ -58,6 +58,8 @@ extern void doesyscmd(const char *); extern void getdivfile(const char *); extern void doformat(const char *[], int); +extern void m4_warnx(const char *, ...); + /* look.c */ #define FLAG_UNTRACED 0 @@ -175,4 +177,5 @@ extern int synch_lines;/* line synchro extern int mimic_gnu; /* behaves like gnu-m4 */ extern int prefix_builtins;/* prefix builtin macros with m4_ */ +extern int fatal_warns;/* make warnings fatal */ Index: usr.bin/m4/gnum4.c === RCS file: /cvs/src/usr.bin/m4/gnum4.c,v retrieving revision 1.50 diff -u -p -u -p -r1.50 gnum4.c --- usr.bin/m4/gnum4.c 29 Apr 2015 00:13:26 - 1.50 +++ usr.bin/m4/gnum4.c 13 Jun 2017 20:22:09 - @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -234,7 +235,7 @@ addchar(int c) } static char * -getstring() +getstring(void) { addchar('\0'); current = 0; @@ -255,11 +256,35 @@ exit_regerror(int er, regex_t *re, const m4errx(1, "regular expression error in %s: %s.", source, errbuf); } +/* warnx() plus check to see if we need to change exit code or exit. + * -E flag functionality. + */ +void +m4_warnx(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + vwarnx(fmt, ap); + va_end(ap); + + /* Do nothing if no -E flags, set exit_code > 0 but keep going +* if one -E flag, exit immediately with exit status > 0 if +* two or more -E flags. +*/ + if (fatal_warns == 0) + return; + else if (fatal_warns == 1) + exit_code = 1; + else + exit(1); +} + static void add_sub(int n, const char *string, regex_t *re, regmatch_t *pm) { if (n > re->re_nsub) - warnx("No subexpression %d", n); + m4_warnx("No subexpression %d", n); /* Subexpressions that did not match are * not an error. */ else if (pm[n].rm_so != -1 && @@ -442,7 +467,7 @@ void dopatsubst(const char *argv[], int argc) { if (argc <= 3) { - warnx("Too few arguments to patsubst"); + m4_warnx("Too few arguments to patsubst"); return; } /* special case: empty regexp */ @@ -494,7 +519,7 @@ doregexp(const char *argv[], int argc) const char *source; if (argc <= 3) { - warnx("Too few arguments to regexp"); + m4_warnx("Too few arguments to regexp"); return; } /* special gnu case */ Index: usr.bin/m4/m4.1 === RCS file: /cvs/src/usr.bin/m4/m4.1,v retrieving revision 1.63 diff -u -p -u -p -r1.63 m4.1 --- usr.bin/m4/m4.1 14 Sep 2015 20:06:58 - 1.63 +++ usr.bin/m4/m4.1 13 Jun 2017 20:22:09 - @@ -38,7 +38,7 @@ .Nd macro language processor .Sh SYNOPSIS .Nm -.Op Fl gPs +.Op Fl EgPs .Oo .Sm off .Fl D Ar name Op No = Ar value @@ -127,6 +127,19 @@ turn on all options. .Pp By default, trace is set to .Qq eq . +.It Fl E +Set warnings to be fatal. +When a single +.Fl E +flag is specified, if warnings are issued, execution continues but +.Nm +will exit with a non-zero exit status. +
Re: m4(1): add -E flag support
On Tue, Jun 13, 2017 at 03:35:54PM -0400, Brian Callahan wrote: > This diff comes with man page additions explaining the new flag, > as well as a regress test for -E -E behavior. I could not figure > out how to write a regress test that checks for exit status, so > there is no test for single -E behavior. You could use the following pattern to test the exit code: if ${M4} -E -E -g ${.CURDIR}/fatalwarnings.m4 >/dev/null 2>&1 || test $$? -ne 1; then false; fi
Re: m4(1): add -E flag support
I don't have an objection to this but wouldn't it be less error-prone to include the check_fatal_warns() checks in a warnx() wrapper? There are some fprintf() calls in tokenizer.l that would need to be converted but that still seems simpler than requiring callers of warnx() or the equivalent to also call check_fatal_warns(). - todd
Re: m4(1): add -E flag support
The decisions taken seem correct. I'll let m4 people judge the rest of it.
m4(1): add -E flag support
Hi tech -- I've shared this with a few developers, and have been advised to share now with a wider audience. This diff adds -E flag functionality to m4(1). I wrote this diff after noticing a patch in ports/devel/scons by jasper@ with the comment: XXX: OpenBSD's m4(1) lacks the -E option (needs to be implemented though). The -E flag causes warnings to become fatal. It appears to be a GNU extension. Unfortunately, the GNU people have multiple definitions of what fatal is, so I will outline the situation and my approach to this diff. The -E flag was first introduced to GNU m4 in 1994, version 1.2. The flag, when set, caused m4 to exit with an exit status > 0 immediately upon issuing its first warning. In GNU m4 1.4.9, released in 2007, this behavior was changed to do the following: 1. If a single -E flag is given, still change the exit status of m4 to be > 0, but otherwise continue as normal. 2. If 2 or more -E flags are given, do the old -E behavior, that is, exit with an exit status > 0 immediately upon issuing the first warning. This is the current behavior of all later GNU m4 releases. I have chosen the 1.4.9 and later behavior for our m4, as it has been now 10 years since the new behavior was introduced. I also looked to see if there is an upstream for m4. There is not as far as I can tell. So we are our own upstream. In fact, we are upstream for other projects as well. Here's that situation: 1. FreeBSD (and DragonFly) sync their m4(1) to ours. They do not have an -E flag because we don't. 2. NetBSD added -E flag support in January 2016. However, NetBSD opted to implement the old pre-GNU m4 1.4.9 -E flag behavior. Additionally, NetBSD's implementation does not error out for all warnings: only warnings for functions contained within gm4.c got the -E flag treatment. Therefore, it is possible to run NetBSD's m4(1) with the -E flag, receive a warning, and not error out (and the -E flag does absolutely nothing in the NetBSD implementation if the -g flag is not also given). My implementation was written entirely independently from the NetBSD implementation, as I did not even know they had added -E flag support until this diff was written and sent off for early review. 3. Solaris and AIX implementations of m4(1) don't have -E flag support. 4. Mac OS X (tested on 10.12.5) has GNU m4 1.4.6 as the version that comes with the base system, meaning that anyone who uses that version will get the old pre-1.4.9 behavior. This diff comes with man page additions explaining the new flag, as well as a regress test for -E -E behavior. I could not figure out how to write a regress test that checks for exit status, so there is no test for single -E behavior. Comments appreciated. ~Brian Index: usr.bin/m4/eval.c === RCS file: /cvs/src/usr.bin/m4/eval.c,v retrieving revision 1.74 diff -u -p -u -p -r1.74 eval.c --- usr.bin/m4/eval.c 5 Feb 2015 12:59:57 - 1.74 +++ usr.bin/m4/eval.c 11 Jun 2017 22:52:08 - @@ -269,6 +269,12 @@ expand_builtin(const char *argv[], int a warn("%s at line %lu: include(%s)", CURRENT_NAME, CURRENT_LINE, argv[2]); exit_code = 1; + /* exit immediately if multiple -E flags +*/ + if (fatal_warns == 2) { + killdiv(); + exit(exit_code); + } } else err(1, "%s at line %lu: include(%s)", CURRENT_NAME, CURRENT_LINE, argv[2]); Index: usr.bin/m4/extern.h === RCS file: /cvs/src/usr.bin/m4/extern.h,v retrieving revision 1.54 diff -u -p -u -p -r1.54 extern.h --- usr.bin/m4/extern.h 12 May 2014 19:11:19 - 1.54 +++ usr.bin/m4/extern.h 11 Jun 2017 22:52:08 - @@ -58,6 +58,8 @@ extern void doesyscmd(const char *); extern void getdivfile(const char *); extern void doformat(const char *[], int); +extern void check_fatal_warns(void); + /* look.c */ #define FLAG_UNTRACED 0 @@ -175,4 +177,5 @@ extern int synch_lines;/* line synchro extern int mimic_gnu; /* behaves like gnu-m4 */ extern int prefix_builtins;/* prefix builtin macros with m4_ */ +extern int fatal_warns;/* make warnings fatal */ Index: usr.bin/m4/gnum4.c === RCS file: /cvs/src/usr.bin/m4/gnum4.c,v retrieving revision 1.50 diff -u -p -u -p -r1.50 gnum4.c --- usr.bin/m4/gnum4.c 29 Apr 2015 00:13:26 - 1.50 +++ usr.bin/m4/gnum4.c 11 Jun 2017 22:52:08 -0