When the first child of the node being validated gets deleted during

validation, man_node_unlink() switches to MAN_NEXT_CHILD.  After
that, we have to switch back to MAN_NEXT_SIBLING after completing
validation, or subsequent parsing would add content into an already
closed node, clobbering potentially existing children, causing
information loss and a memory leak.  Bug found by kristaps@ with
valgrind in groff(7) on Mac OS X.

Note that the switch back must be conditional, for if the node being
validated itself gets deleted, we must *not* go to MAN_NEXT_SIBLING,
which would not only yield wrong results in general but also crash
in malformed manuals having an empty paragraph before the first .SH,
for example OpenBSD c++filt(1).

While here, add the missing <sys/types.h> as required before mandoc.h.

diff --git a/regress/usr.bin/mandoc/man/RS/Makefile b/regress/usr.bin/mandoc/man/RS/Makefile
index 66fdf8d..40041f7 100644 (file)
--- a/regress/usr.bin/mandoc/man/RS/Makefile
+++ b/regress/usr.bin/mandoc/man/RS/Makefile
@@ -1,6 +1,11 @@
-# $OpenBSD: Makefile,v 1.7 2014/07/07 21:35:42 schwarze Exp $
+# $OpenBSD: Makefile,v 1.8 2014/08/18 16:26:13 schwarze Exp $
 
-REGRESS_TARGETS        = breaking empty literal lonelyRE nested noRE width
-LINT_TARGETS   = lonelyRE noRE
+REGRESS_TARGETS         = breaking broken empty literal lonelyRE nested noRE width
+LINT_TARGETS    = broken lonelyRE noRE
+
+# groff-1.22.2/mandoc difference:
+# RS PP RE gives a blank line in groff, none in mandoc.
+
+SKIP_GROFF      = broken
 
 .include <bsd.regress.mk>

diff --git a/regress/usr.bin/mandoc/man/RS/broken.in b/regress/usr.bin/mandoc/man/RS/broken.in
new file mode 100644 (file)
index 0000000..b1ca9e3
--- /dev/null
+++ b/regress/usr.bin/mandoc/man/RS/broken.in
@@ -0,0 +1,15 @@
+.TH RS-BROKEN 1 "August 28, 2014" OpenBSD
+.SH NAME
+RS-broken \- indented blocks broken by other blocks
+.SH DESCRIPTION
+initial text
+.RS 2n
+indented
+.PP
+still indented
+.RE
+back to normal
+.RS
+.P
+.RE
+final text

diff --git a/regress/usr.bin/mandoc/man/RS/broken.out_ascii b/regress/usr.bin/mandoc/man/RS/broken.out_ascii
new file mode 100644 (file)
index 0000000..ce648d1
--- /dev/null
+++ b/regress/usr.bin/mandoc/man/RS/broken.out_ascii
@@ -0,0 +1,18 @@
+RS-BROKEN(1)               OpenBSD Reference Manual               RS-BROKEN(1)
+
+
+
+N\bNA\bAM\bME\bE
+       RS-broken - indented blocks broken by other blocks
+
+D\bDE\bES\bSC\bCR\bRI\bIP\bPT\bTI\bIO\bON\bN
+       initial text
+         indented
+
+         still indented
+       back to normal
+       final text
+
+
+
+OpenBSD                         August 28, 2014                   RS-BROKEN(1)

diff --git a/regress/usr.bin/mandoc/man/RS/broken.out_lint b/regress/usr.bin/mandoc/man/RS/broken.out_lint
new file mode 100644 (file)
index 0000000..8286173
--- /dev/null
+++ b/regress/usr.bin/mandoc/man/RS/broken.out_lint
@@ -0,0 +1,2 @@
+mandoc: broken.in:13:2: WARNING: skipping paragraph macro: P empty
+mandoc: broken.in:12:2: WARNING: argument count wrong: want children (have none)

diff --git a/regress/usr.bin/mandoc/man/SH/Makefile b/regress/usr.bin/mandoc/man/SH/Makefile
index 88123c3..b4ce68b 100644 (file)
--- a/regress/usr.bin/mandoc/man/SH/Makefile
+++ b/regress/usr.bin/mandoc/man/SH/Makefile
@@ -1,7 +1,13 @@
-# $OpenBSD: Makefile,v 1.1 2014/08/14 02:00:52 schwarze Exp $
+# $OpenBSD: Makefile,v 1.2 2014/08/18 16:26:13 schwarze Exp $
 
-REGRESS_TARGETS        = broken broken_eline noarg
-LINT_TARGETS   = broken broken_eline noarg
+REGRESS_TARGETS        = broken broken_eline empty_before noarg
+LINT_TARGETS   = broken broken_eline empty_before noarg
+
+# groff-1.22.2/mandoc differences:
+#  * .SH without arguments just before EOF causes
+#    two additional blank lines in groff.  [broken broken_eline]
+#  * Vertical spacing is completely different
+#    for empty .SH and .SS heads.  [noarg]
 
 SKIP_GROFF     = broken broken_eline noarg
 

diff --git a/regress/usr.bin/mandoc/man/SH/empty_before.in b/regress/usr.bin/mandoc/man/SH/empty_before.in
new file mode 100644 (file)
index 0000000..af1e4ac
--- /dev/null
+++ b/regress/usr.bin/mandoc/man/SH/empty_before.in
@@ -0,0 +1,6 @@
+.TH SH-EMPTY_BEFORE 1 "August 18, 2014" OpenBSD
+.PP
+.SH NAME
+SH-empty_before \- empty paragraph before first section header
+.SH DESCRIPTION
+some text

diff --git a/regress/usr.bin/mandoc/man/SH/empty_before.out_ascii b/regress/usr.bin/mandoc/man/SH/empty_before.out_ascii
new file mode 100644 (file)
index 0000000..045ef69
--- /dev/null
+++ b/regress/usr.bin/mandoc/man/SH/empty_before.out_ascii
@@ -0,0 +1,13 @@
+SH-EMPTY_BEFORE(1)         OpenBSD Reference Manual         SH-EMPTY_BEFORE(1)
+
+
+
+N\bNA\bAM\bME\bE
+       SH-empty_before - empty paragraph before first section header
+
+D\bDE\bES\bSC\bCR\bRI\bIP\bPT\bTI\bIO\bON\bN
+       some text
+
+
+
+OpenBSD                         August 18, 2014             SH-EMPTY_BEFORE(1)

diff --git a/regress/usr.bin/mandoc/man/SH/empty_before.out_lint b/regress/usr.bin/mandoc/man/SH/empty_before.out_lint
new file mode 100644 (file)
index 0000000..b10c421
--- /dev/null
+++ b/regress/usr.bin/mandoc/man/SH/empty_before.out_lint
@@ -0,0 +1 @@
+mandoc: empty_before.in:2:2: WARNING: skipping paragraph macro: PP empty

diff --git a/usr.bin/mandoc/man_macro.c b/usr.bin/mandoc/man_macro.c
index 737aa0d..76b35f7 100644 (file)
--- a/usr.bin/mandoc/man_macro.c
+++ b/usr.bin/mandoc/man_macro.c
@@ -1,4 +1,4 @@
-/*     $Id: man_macro.c,v 1.50 2014/08/08 15:35:31 schwarze Exp $ */
+/*     $Id: man_macro.c,v 1.51 2014/08/18 16:26:13 schwarze Exp $ */
 /*
  * Copyright (c) 2008, 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2012, 2013 Ingo Schwarze <schwarze@openbsd.org>
@@ -16,6 +16,9 @@
  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
+
+#include <sys/types.h>
+
 #include <assert.h>
 #include <ctype.h>
 #include <stdlib.h>
@@ -96,7 +99,6 @@ man_unscope(struct man *man, const struct man_node *to)
 {
        struct man_node *n;
 
-       man->next = MAN_NEXT_SIBLING;
        to = to->parent;
        n = man->last;
        while (n != to) {
@@ -135,11 +137,23 @@ man_unscope(struct man *man, const struct man_node *to)
                 * Save a pointer to the parent such that
                 * we know where to continue the iteration.
                 */
+
                man->last = n;
                n = n->parent;
                if ( ! man_valid_post(man))
                        return(0);
        }
+
+       /*
+        * If we ended up at the parent of the node we were
+        * supposed to rewind to, that means the target node
+        * got deleted, so add the next node we parse as a child
+        * of the parent instead of as a sibling of the target.
+        */
+
+       man->next = (man->last == to) ?
+           MAN_NEXT_CHILD : MAN_NEXT_SIBLING;
+
        return(1);
 }