[PATCH] show error level on stderr

[PATCH] show error level on stderr

[Ingo Schwarze]

Hi,

while looking into nesting woes, i wished to be able to distinguish
warnings, errors and fatal errors without looking at mandoc.h and
main.c.  So i decided to add FATAL: and ERROR: to the error messages
on stdout, leaving warnings alone.  While writing the patch, i
noticed that nomenclature is inconsistent: with_error refers to
MANDOCERR_FATAL.  Besides, we decided in Elmenhorst to return
an error code when encountering an error; in fact, the code still
exits non-zero even when only a warning is encountered.

Thus, the following patch
 * renames with_error to with_fatal
 * tracks with_error instead of with_warning
 * uses these two to determine the exit code
 * adds ERROR: and FATAL: to messages
 * sorts the code in mmsg() to make it easier on the eye

OK?

Yours,
  Ingo


Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/main.c,v
retrieving revision 1.39
diff -u -p -r1.39 main.c
--- main.c	29 Jun 2010 15:49:52 -0000	1.39
+++ main.c	30 Jun 2010 17:37:01 -0000
@@ -171,8 +171,8 @@ static	void		  version(void) __attribute
 static	int		  woptions(int *, char *);
 
 static	const char	 *progname;
-static 	int		  with_error;
-static	int		  with_warning;
+static	int		  with_fatal;
+static	int		  with_error;
 
 int
 main(int argc, char *argv[])
@@ -235,7 +235,7 @@ main(int argc, char *argv[])
 	while (*argv) {
 		ffile(*argv, &curp);
 
-		if (with_error && !(curp.fflags & FL_IGN_ERRORS))
+		if (with_fatal && !(curp.fflags & FL_IGN_ERRORS))
 			break;
 		++argv;
 	}
@@ -249,7 +249,7 @@ main(int argc, char *argv[])
 	if (curp.roff)
 		roff_free(curp.roff);
 
-	return((with_warning || with_error) ? 
+	return((with_fatal || with_error) ? 
 			EXIT_FAILURE :  EXIT_SUCCESS);
 }
 
@@ -327,7 +327,7 @@ ffile(const char *file, struct curparse 
 	curp->file = file;
 	if (-1 == (curp->fd = open(curp->file, O_RDONLY, 0))) {
 		perror(curp->file);
-		with_error = 1;
+		with_fatal = 1;
 		return;
 	}
 
@@ -368,7 +368,7 @@ read_whole_file(struct curparse *curp, s
 
 	if (-1 == fstat(curp->fd, &st)) {
 		perror(curp->file);
-		with_error = 1;
+		with_fatal = 1;
 		return(0);
 	}
 
@@ -383,7 +383,7 @@ read_whole_file(struct curparse *curp, s
 		if (st.st_size >= (1U << 31)) {
 			fprintf(stderr, "%s: input too large\n", 
 					curp->file);
-			with_error = 1;
+			with_fatal = 1;
 			return(0);
 		}
 		*with_mmap = 1;
@@ -427,7 +427,7 @@ read_whole_file(struct curparse *curp, s
 
 	free(fb->buf);
 	fb->buf = NULL;
-	with_error = 1;
+	with_fatal = 1;
 	return(0);
 }
 
@@ -643,7 +643,7 @@ fdesc(struct curparse *curp)
 	return;
 
  bailout:
-	with_error = 1;
+	with_fatal = 1;
 	goto cleanup;
 }
 
@@ -828,30 +828,37 @@ static int
 mmsg(enum mandocerr t, void *arg, int ln, int col, const char *msg)
 {
 	struct curparse *cp;
+	const char *level;
+	int rc;
 
 	cp = (struct curparse *)arg;
+	level = NULL;
+	rc = 1;
 
-	if (t <= MANDOCERR_ERROR) {
-		if ( ! (cp->wflags & WARN_WALL))
+	if (t > MANDOCERR_ERROR) {
+		with_fatal = 1;
+		level = "FATAL";
+		rc = 0;
+	} else {
+		if ( ! (WARN_WALL & cp->wflags))
 			return(1);
-		with_warning = 1;
-	} else
-		with_error = 1;
-
-	fprintf(stderr, "%s:%d:%d: %s", cp->file, 
-			ln, col + 1, mandocerrs[t]);
+		if (t > MANDOCERR_WARNING) {
+			with_error = 1;
+			level = "ERROR";
+		}
+		if (WARN_WERR & cp->wflags) {
+			with_fatal = 1;
+			rc = 0;
+		}
+	}
 
+	fprintf(stderr, "%s:%d:%d:", cp->file, ln, col + 1);
+	if (level)
+		fprintf(stderr, " %s:", level);
+	fprintf(stderr, " %s", mandocerrs[t]);
 	if (msg)
 		fprintf(stderr, ": %s", msg);
-
 	fputc('\n', stderr);
 
-	/* This is superfluous, but whatever. */
-	if (t > MANDOCERR_ERROR)
-		return(0);
-	if (cp->wflags & WARN_WERR) {
-		with_error = 1;
-		return(0);
-	}
-	return(1);
+	return(rc);
 }
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: [PATCH] show error level on stderr

[Ingo Schwarze]

Hi,

here is the same patch again, adding a few more simple improvements:
 * make MANDOCERR_WARNING, MANDOCERR_ERROR, MANDOCERR_FATAL
   real enum values, not #defines, such that adding and removing
   messages becomes easier, and such that you do not need to
   remember whether to use > or >= operators
 * move them to the top of the respective lists of messages,
   such that they look like titles, and such that you can use
   the intuitive notation "t >= MANDOCERR_FATAL"
 * use blank lines to make the message lists more readable
 * update a few comments in mandoc.h

OK?


Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/main.c,v
retrieving revision 1.39
diff -u -p -r1.39 main.c
--- main.c	29 Jun 2010 15:49:52 -0000	1.39
+++ main.c	30 Jun 2010 18:36:37 -0000
@@ -88,6 +88,9 @@ struct	curparse {
 
 static	const char * const	mandocerrs[MANDOCERR_MAX] = {
 	"ok",
+
+	"generic warning",
+
 	"text should be uppercase",
 	"sections out of conventional order",
 	"section name repeats",
@@ -107,6 +110,9 @@ static	const char * const	mandocerrs[MAN
 	"section not in conventional manual section",
 	"end of line whitespace",
 	"scope open on exit",
+
+	"generic error",
+
 	"NAME section must come first",
 	"bad Boolean value",
 	"child violates parent syntax",
@@ -138,6 +144,9 @@ static	const char * const	mandocerrs[MAN
 	"missing display type",
 	"line argument(s) will be lost",
 	"body argument(s) will be lost",
+
+	"generic fatal error",
+
 	"column syntax is inconsistent",
 	"missing font type",
 	"displays may not be nested",
@@ -171,8 +180,8 @@ static	void		  version(void) __attribute
 static	int		  woptions(int *, char *);
 
 static	const char	 *progname;
-static 	int		  with_error;
-static	int		  with_warning;
+static	int		  with_fatal;
+static	int		  with_error;
 
 int
 main(int argc, char *argv[])
@@ -235,7 +244,7 @@ main(int argc, char *argv[])
 	while (*argv) {
 		ffile(*argv, &curp);
 
-		if (with_error && !(curp.fflags & FL_IGN_ERRORS))
+		if (with_fatal && !(curp.fflags & FL_IGN_ERRORS))
 			break;
 		++argv;
 	}
@@ -249,7 +258,7 @@ main(int argc, char *argv[])
 	if (curp.roff)
 		roff_free(curp.roff);
 
-	return((with_warning || with_error) ? 
+	return((with_fatal || with_error) ? 
 			EXIT_FAILURE :  EXIT_SUCCESS);
 }
 
@@ -327,7 +336,7 @@ ffile(const char *file, struct curparse 
 	curp->file = file;
 	if (-1 == (curp->fd = open(curp->file, O_RDONLY, 0))) {
 		perror(curp->file);
-		with_error = 1;
+		with_fatal = 1;
 		return;
 	}
 
@@ -368,7 +377,7 @@ read_whole_file(struct curparse *curp, s
 
 	if (-1 == fstat(curp->fd, &st)) {
 		perror(curp->file);
-		with_error = 1;
+		with_fatal = 1;
 		return(0);
 	}
 
@@ -383,7 +392,7 @@ read_whole_file(struct curparse *curp, s
 		if (st.st_size >= (1U << 31)) {
 			fprintf(stderr, "%s: input too large\n", 
 					curp->file);
-			with_error = 1;
+			with_fatal = 1;
 			return(0);
 		}
 		*with_mmap = 1;
@@ -427,7 +436,7 @@ read_whole_file(struct curparse *curp, s
 
 	free(fb->buf);
 	fb->buf = NULL;
-	with_error = 1;
+	with_fatal = 1;
 	return(0);
 }
 
@@ -643,7 +652,7 @@ fdesc(struct curparse *curp)
 	return;
 
  bailout:
-	with_error = 1;
+	with_fatal = 1;
 	goto cleanup;
 }
 
@@ -828,30 +837,37 @@ static int
 mmsg(enum mandocerr t, void *arg, int ln, int col, const char *msg)
 {
 	struct curparse *cp;
+	const char *level;
+	int rc;
 
 	cp = (struct curparse *)arg;
+	level = NULL;
+	rc = 1;
 
-	if (t <= MANDOCERR_ERROR) {
-		if ( ! (cp->wflags & WARN_WALL))
+	if (t >= MANDOCERR_FATAL) {
+		with_fatal = 1;
+		level = "FATAL";
+		rc = 0;
+	} else {
+		if ( ! (WARN_WALL & cp->wflags))
 			return(1);
-		with_warning = 1;
-	} else
-		with_error = 1;
-
-	fprintf(stderr, "%s:%d:%d: %s", cp->file, 
-			ln, col + 1, mandocerrs[t]);
+		if (t >= MANDOCERR_ERROR) {
+			with_error = 1;
+			level = "ERROR";
+		}
+		if (WARN_WERR & cp->wflags) {
+			with_fatal = 1;
+			rc = 0;
+		}
+	}
 
+	fprintf(stderr, "%s:%d:%d:", cp->file, ln, col + 1);
+	if (level)
+		fprintf(stderr, " %s:", level);
+	fprintf(stderr, " %s", mandocerrs[t]);
 	if (msg)
 		fprintf(stderr, ": %s", msg);
-
 	fputc('\n', stderr);
 
-	/* This is superfluous, but whatever. */
-	if (t > MANDOCERR_ERROR)
-		return(0);
-	if (cp->wflags & WARN_WERR) {
-		with_error = 1;
-		return(0);
-	}
-	return(1);
+	return(rc);
 }
Index: mandoc.h
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mandoc.h,v
retrieving revision 1.7
diff -u -p -r1.7 mandoc.h
--- mandoc.h	26 Jun 2010 17:56:43 -0000	1.7
+++ mandoc.h	30 Jun 2010 18:36:37 -0000
@@ -25,6 +25,9 @@ __BEGIN_DECLS
 
 enum	mandocerr {
 	MANDOCERR_OK,
+
+	MANDOCERR_WARNING, /* ===== end of warnings ===== */
+
 	MANDOCERR_UPPERCASE, /* text should be uppercase */
 	MANDOCERR_SECOOO, /* sections out of conventional order */
 	MANDOCERR_SECREP, /* section name repeats */
@@ -38,14 +41,15 @@ enum	mandocerr {
 	MANDOCERR_NOWIDTHARG, /* argument requires the width argument */
 	/* FIXME: merge with MANDOCERR_IGNARGV. */
 	MANDOCERR_WIDTHARG, /* superfluous width argument */
-	MANDOCERR_IGNARGV, /* macro ignoring argv */
+	MANDOCERR_IGNARGV, /* ignoring argument */
 	MANDOCERR_BADDATE, /* bad date argument */
 	MANDOCERR_BADWIDTH, /* bad width argument */
 	MANDOCERR_BADMSEC, /* unknown manual section */
 	MANDOCERR_SECMSEC, /* section not in conventional manual section */
 	MANDOCERR_EOLNSPACE, /* end of line whitespace */
 	MANDOCERR_SCOPEEXIT, /* scope open on exit */
-#define	MANDOCERR_WARNING	MANDOCERR_SCOPEEXIT
+
+	MANDOCERR_ERROR, /* ===== end of errors ===== */
 
 	MANDOCERR_NAMESECFIRST, /* NAME section must come first */
 	MANDOCERR_BADBOOL, /* bad Boolean value */
@@ -79,7 +83,8 @@ enum	mandocerr {
 	MANDOCERR_DISPTYPE, /* missing display type */
 	MANDOCERR_ARGSLOST, /* line argument(s) will be lost */
 	MANDOCERR_BODYLOST, /* body argument(s) will be lost */
-#define	MANDOCERR_ERROR		MANDOCERR_BODYLOST
+
+	MANDOCERR_FATAL, /* ===== end of fatal errors ===== */
 
 	MANDOCERR_COLUMNS, /* column syntax is inconsistent */
 	/* FIXME: this should be a MANDOCERR_ERROR */
@@ -87,7 +92,7 @@ enum	mandocerr {
 	/* FIXME: this should be a MANDOCERR_ERROR */
 	MANDOCERR_NESTEDDISP, /* displays may not be nested */
 	MANDOCERR_BADDISP, /* unsupported display type */
-	MANDOCERR_SYNTNOSCOPE, /* request scope close w/none open */
+	MANDOCERR_SYNTNOSCOPE, /* no scope to rewind: syntax violated */
 	MANDOCERR_SYNTSCOPE, /* scope broken, syntax violated */
 	MANDOCERR_SYNTLINESCOPE, /* line scope broken, syntax violated */
 	MANDOCERR_SYNTARGVCOUNT, /* argument count wrong, violates syntax */
@@ -95,9 +100,8 @@ enum	mandocerr {
 	MANDOCERR_SYNTARGCOUNT, /* argument count wrong, violates syntax */
 	MANDOCERR_NODOCBODY, /* no document body */
 	MANDOCERR_NODOCPROLOG, /* no document prologue */
-	MANDOCERR_UTSNAME, /* utsname() system call failed */
+	MANDOCERR_UTSNAME, /* utsname system call failed */
 	MANDOCERR_MEM, /* memory exhausted */
-#define	MANDOCERR_FATAL		MANDOCERR_MEM
 
 	MANDOCERR_MAX
 };
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: [PATCH] show error level on stderr

[Kristaps Dzonsons]

Ingo, this is fine by me.

Thanks,

Kristaps
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv