Parse for the closing delimiter `\}' for conditionals

even when the conditional evaluated to false.

While here, reshuffle the code to reduce indentation and make it
more readable; that way, we can even trim down the comments because
it becomes obvious what the code does.

Found in zipinfo(1) - thanks to espie@ and naddy@
for making me look at that manual page.

4 files changed, 63 insertions, 46 deletions

diff --git a/regress/usr.bin/mandoc/roff/cond/Makefile b/regress/usr.bin/mandoc/roff/cond/Makefile
index 600e989078e..aea6c49fe38 100644
--- a/regress/usr.bin/mandoc/roff/cond/Makefile
+++ b/regress/usr.bin/mandoc/roff/cond/Makefile
@@ -1,5 +1,5 @@
-# $OpenBSD: Makefile,v 1.2 2012/05/27 14:35:40 schwarze Exp $
+# $OpenBSD: Makefile,v 1.3 2013/06/27 09:48:23 schwarze Exp $
 
-REGRESS_TARGETS=if ie before-Dd
+REGRESS_TARGETS=if ie close before-Dd
 
 .include <bsd.regress.mk>
diff --git a/regress/usr.bin/mandoc/roff/cond/close.in b/regress/usr.bin/mandoc/roff/cond/close.in
new file mode 100644
index 00000000000..af30596391f
--- /dev/null
+++ b/regress/usr.bin/mandoc/roff/cond/close.in
@@ -0,0 +1,12 @@
+.TH COND-CLOSE 1 2013-06-27 OpenBSD
+.SH NAME
+cond-close \- closing conditional macros
+.SH DESCRIPTION
+.if t \{.unknown \}
+closing after an unknown macro
+.PP
+.if t \{.ig \}
+closing after an ignored macro
+.PP
+.if t \{text \}
+closing after plain text
diff --git a/regress/usr.bin/mandoc/roff/cond/close.out_ascii b/regress/usr.bin/mandoc/roff/cond/close.out_ascii
new file mode 100644
index 00000000000..efe79fae324
--- /dev/null
+++ b/regress/usr.bin/mandoc/roff/cond/close.out_ascii
@@ -0,0 +1,17 @@
+COND-CLOSE(1)              OpenBSD Reference Manual              COND-CLOSE(1)
+
+
+
+NNAAMMEE
+       cond-close - closing conditional macros
+
+DDEESSCCRRIIPPTTIIOONN
+       closing after an unknown macro
+
+       closing after an ignored macro
+
+       closing after plain text
+
+
+
+OpenBSD                           2013-06-27                     COND-CLOSE(1)
diff --git a/usr.bin/mandoc/roff.c b/usr.bin/mandoc/roff.c
index 906331e15bb..c6341b52fa4 100644
--- a/usr.bin/mandoc/roff.c
+++ b/usr.bin/mandoc/roff.c
@@ -1,7 +1,7 @@
-/*	$Id: roff.c,v 1.49 2013/05/31 22:08:03 schwarze Exp $ */
+/*	$Id: roff.c,v 1.50 2013/06/27 09:48:22 schwarze Exp $ */
 /*
  * Copyright (c) 2010, 2011, 2012 Kristaps Dzonsons <kristaps@bsd.lv>
- * Copyright (c) 2010, 2011, 2012 Ingo Schwarze <schwarze@openbsd.org>
+ * Copyright (c) 2010, 2011, 2012, 2013 Ingo Schwarze <schwarze@openbsd.org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -1024,57 +1024,45 @@ roff_cond_sub(ROFF_ARGS)
 
 	rr = r->last->rule;
 	roffnode_cleanscope(r);
+	t = roff_parse(r, *bufp, &pos);
 
 	/*
-	 * If the macro is unknown, first check if it contains a closing
-	 * delimiter `\}'.  If it does, close out our scope and return
-	 * the currently-scoped rule (ignore or continue).  Else, drop
-	 * into the currently-scoped rule.
+	 * Fully handle known macros when they are structurally
+	 * required or when the conditional evaluated to true.
 	 */
 
-	if (ROFF_MAX == (t = roff_parse(r, *bufp, &pos))) {
-		ep = &(*bufp)[pos];
-		for ( ; NULL != (ep = strchr(ep, '\\')); ep++) {
-			ep++;
-			if ('}' != *ep)
-				continue;
-
-			/*
-			 * Make the \} go away.
-			 * This is a little haphazard, as it's not quite
-			 * clear how nroff does this.
-			 * If we're at the end of line, then just chop
-			 * off the \} and resize the buffer.
-			 * If we aren't, then conver it to spaces.
-			 */
+	if ((ROFF_MAX != t) &&
+	    (ROFF_ccond == t || ROFFRULE_ALLOW == rr ||
+	     ROFFMAC_STRUCT & roffs[t].flags)) {
+		assert(roffs[t].proc);
+		return((*roffs[t].proc)(r, t, bufp, szp,
+					ln, ppos, pos, offs));
+	}
 
-			if ('\0' == *(ep + 1)) {
-				*--ep = '\0';
-				*szp -= 2;
-			} else
-				*(ep - 1) = *ep = ' ';
+	/* Always check for the closing delimiter `\}'. */
 
-			roff_ccond(r, ROFF_ccond, bufp, szp, 
-					ln, pos, pos + 2, offs);
-			break;
-		}
-		return(ROFFRULE_DENY == rr ? ROFF_IGN : ROFF_CONT);
-	}
+	ep = &(*bufp)[pos];
+	while (NULL != (ep = strchr(ep, '\\'))) {
+		if ('}' != *(++ep))
+			continue;
 
-	/*
-	 * A denied conditional must evaluate its children if and only
-	 * if they're either structurally required (such as loops and
-	 * conditionals) or a closing macro.
-	 */
+		/*
+		 * If we're at the end of line, then just chop
+		 * off the \} and resize the buffer.
+		 * If we aren't, then convert it to spaces.
+		 */
 
-	if (ROFFRULE_DENY == rr)
-		if ( ! (ROFFMAC_STRUCT & roffs[t].flags))
-			if (ROFF_ccond != t)
-				return(ROFF_IGN);
+		if ('\0' == *(ep + 1)) {
+			*--ep = '\0';
+			*szp -= 2;
+		} else
+			*(ep - 1) = *ep = ' ';
 
-	assert(roffs[t].proc);
-	return((*roffs[t].proc)(r, t, bufp, szp, 
-				ln, ppos, pos, offs));
+		roff_ccond(r, ROFF_ccond, bufp, szp, 
+				ln, pos, pos + 2, offs);
+		break;
+	}
+	return(ROFFRULE_DENY == rr ? ROFF_IGN : ROFF_CONT);
 }
 
 /* ARGSUSED */