Clean up the validation of .Pp, .PP, .sp, and .br. Make sure all

combinations are handled, and are handled in a systematic manner.
This resolves some erratic duplicate handling, handles a number of
missing cases, and improves diagnostics in various respects.

Move validation of .br and .sp to the roff validation module
rather than doing that twice in the mdoc and man validation modules.
Move the node relinking function to the roff library where it belongs.

In validation functions, only look at the node itself, at previous
nodes, and at descendants, not at following nodes or ancestors,
such that only nodes are inspected which are already validated.

1 files changed, 14 insertions, 41 deletions

diff --git a/usr.bin/mandoc/mdoc_validate.c b/usr.bin/mandoc/mdoc_validate.c
index 1c80db2b30d..61cdc5b96e6 100644
--- a/usr.bin/mandoc/mdoc_validate.c
+++ b/usr.bin/mandoc/mdoc_validate.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mdoc_validate.c,v 1.278 2018/12/03 21:00:06 schwarze Exp $ */
+/*	$OpenBSD: mdoc_validate.c,v 1.279 2018/12/04 02:53:45 schwarze Exp $ */
 /*
  * Copyright (c) 2008-2012 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2010-2018 Ingo Schwarze <schwarze@openbsd.org>
@@ -368,15 +368,7 @@ mdoc_node_validate(struct roff_man *mdoc)
 		/* Call the macro's postprocessor. */
 
 		if (n->tok < ROFF_MAX) {
-			switch(n->tok) {
-			case ROFF_br:
-			case ROFF_sp:
-				post_par(mdoc);
-				break;
-			default:
-				roff_validate(mdoc);
-				break;
-			}
+			roff_validate(mdoc);
 			break;
 		}
 
@@ -954,7 +946,7 @@ build_list(struct roff_man *mdoc, int tok)
 	for (ic = 1;; ic++) {
 		roff_elem_alloc(mdoc, n->line, n->pos, tok);
 		mdoc->last->flags |= NODE_NOSRC;
-		mdoc_node_relink(mdoc, n);
+		roff_node_relink(mdoc, n);
 		n = mdoc->last = mdoc->last->parent;
 		mdoc->next = ROFF_NEXT_SIBLING;
 		if (n->next == NULL)
@@ -1282,7 +1274,7 @@ post_nm(POST_ARGS)
 		mandoc_xr_add(mdoc->meta.msec, n->child->string, -1, -1);
 
 	if (n->last != NULL && n->last->tok == MDOC_Pp)
-		mdoc_node_relink(mdoc, n->last);
+		roff_node_relink(mdoc, n->last);
 
 	if (mdoc->meta.name == NULL)
 		deroff(&mdoc->meta.name, n);
@@ -1361,7 +1353,7 @@ post_display(POST_ARGS)
 				    mdoc->parse, n->line, n->pos, "Bd");
 				mdoc->next = ROFF_NEXT_SIBLING;
 				while (n->body->child != NULL)
-					mdoc_node_relink(mdoc,
+					roff_node_relink(mdoc,
 					    n->body->child);
 				roff_node_delete(mdoc, n);
 				break;
@@ -1644,7 +1636,7 @@ post_bl_block(POST_ARGS)
 				mandoc_msg(MANDOCERR_PAR_MOVE,
 				    mdoc->parse, nc->line, nc->pos,
 				    roff_name[nc->tok]);
-				mdoc_node_relink(mdoc, nc);
+				roff_node_relink(mdoc, nc);
 			} else if (n->norm->Bl.comp == 0 &&
 			    n->norm->Bl.type != LIST_column) {
 				mandoc_vmsg(MANDOCERR_PAR_SKIP,
@@ -1804,7 +1796,7 @@ post_bl(POST_ARGS)
 			roff_body_alloc(mdoc, nchild->line,
 			    nchild->pos, MDOC_It);
 			while (nchild->tok != MDOC_It) {
-				mdoc_node_relink(mdoc, nchild);
+				roff_node_relink(mdoc, nchild);
 				if ((nchild = nnext) == NULL)
 					break;
 				nnext = nchild->next;
@@ -1923,7 +1915,7 @@ post_sm(POST_ARGS)
 	mandoc_vmsg(MANDOCERR_SM_BAD,
 	    mdoc->parse, nch->line, nch->pos,
 	    "%s %s", roff_name[mdoc->last->tok], nch->string);
-	mdoc_node_relink(mdoc, nch);
+	roff_node_relink(mdoc, nch);
 	return;
 }
 
@@ -2507,7 +2499,8 @@ post_ignpar(POST_ARGS)
 	}
 
 	if ((np = mdoc->last->child) != NULL)
-		if (np->tok == MDOC_Pp) {
+		if (np->tok == MDOC_Pp ||
+		    np->tok == ROFF_br || np->tok == ROFF_sp) {
 			mandoc_vmsg(MANDOCERR_PAR_SKIP,
 			    mdoc->parse, np->line, np->pos,
 			    "%s after %s", roff_name[np->tok],
@@ -2516,7 +2509,7 @@ post_ignpar(POST_ARGS)
 		}
 
 	if ((np = mdoc->last->last) != NULL)
-		if (np->tok == MDOC_Pp) {
+		if (np->tok == MDOC_Pp || np->tok == ROFF_br) {
 			mandoc_vmsg(MANDOCERR_PAR_SKIP, mdoc->parse,
 			    np->line, np->pos, "%s at the end of %s",
 			    roff_name[np->tok],
@@ -2561,33 +2554,13 @@ post_par(POST_ARGS)
 {
 	struct roff_node *np;
 
-	np = mdoc->last;
-	if (np->tok != ROFF_br && np->tok != ROFF_sp)
-		post_prevpar(mdoc);
+	post_prevpar(mdoc);
 
-	if (np->tok == ROFF_sp) {
-		if (np->child != NULL && np->child->next != NULL)
-			mandoc_vmsg(MANDOCERR_ARG_EXCESS, mdoc->parse,
-			    np->child->next->line, np->child->next->pos,
-			    "sp ... %s", np->child->next->string);
-	} else if (np->child != NULL)
+	np = mdoc->last;
+	if (np->child != NULL)
 		mandoc_vmsg(MANDOCERR_ARG_SKIP,
 		    mdoc->parse, np->line, np->pos, "%s %s",
 		    roff_name[np->tok], np->child->string);
-
-	if ((np = mdoc->last->prev) == NULL) {
-		np = mdoc->last->parent;
-		if (np->tok != MDOC_Sh && np->tok != MDOC_Ss)
-			return;
-	} else if (np->tok != MDOC_Pp &&
-	    (mdoc->last->tok != ROFF_br ||
-	     (np->tok != ROFF_sp && np->tok != ROFF_br)))
-		return;
-
-	mandoc_vmsg(MANDOCERR_PAR_SKIP, mdoc->parse,
-	    mdoc->last->line, mdoc->last->pos, "%s after %s",
-	    roff_name[mdoc->last->tok], roff_name[np->tok]);
-	roff_node_delete(mdoc, mdoc->last);
 }
 
 static void