Re: mdocml: Convert an assert into an explicit check.

Re: mdocml: Convert an assert into an explicit check.

[Ingo Schwarze]

Hi Joerg,

joerg@mdocml.bsd.lv wrote on Fri, Nov 18, 2011 at 11:39:08AM -0500:

> Log Message:
> -----------
> Convert an assert into an explicit check. man_unscope can be triggered
> on unknown macros.
> 
> Modified Files:
> --------------
>     mdocml:
>         man_macro.c

Sorry, so far i failed to reproduce the problem.
Can you provide an example?

Thanks,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Convert an assert into an explicit check.

[Joerg Sonnenberger]

On Sat, Nov 19, 2011 at 03:19:07PM +0100, Ingo Schwarze wrote:
> Hi Joerg,
> 
> joerg@mdocml.bsd.lv wrote on Fri, Nov 18, 2011 at 11:39:08AM -0500:
> 
> > Log Message:
> > -----------
> > Convert an assert into an explicit check. man_unscope can be triggered
> > on unknown macros.
> > 
> > Modified Files:
> > --------------
> >     mdocml:
> >         man_macro.c
> 
> Sorry, so far i failed to reproduce the problem.
> Can you provide an example?

.ab help2man is required to generate this page

was the case that originally triggered this.

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

Re: mdocml: Convert an assert into an explicit check.

[Joerg Sonnenberger]

On Sun, Nov 20, 2011 at 05:25:12AM +0100, Joerg Sonnenberger wrote:
> On Sat, Nov 19, 2011 at 03:19:07PM +0100, Ingo Schwarze wrote:
> > Hi Joerg,
> > 
> > joerg@mdocml.bsd.lv wrote on Fri, Nov 18, 2011 at 11:39:08AM -0500:
> > 
> > > Log Message:
> > > -----------
> > > Convert an assert into an explicit check. man_unscope can be triggered
> > > on unknown macros.
> > > 
> > > Modified Files:
> > > --------------
> > >     mdocml:
> > >         man_macro.c
> > 
> > Sorry, so far i failed to reproduce the problem.
> > Can you provide an example?
> 
> .ab help2man is required to generate this page
> 
> was the case that originally triggered this.

I think this also triggers the other case with missing default settings
in man_term.c later.

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

Re: mdocml: Convert an assert into an explicit check.

[Ingo Schwarze]

Hi Joerg,

Joerg Sonnenberger wrote on Sun, Nov 20, 2011 at 05:26:44AM +0100:
> On Sun, Nov 20, 2011 at 05:25:12AM +0100, Joerg Sonnenberger wrote:
>> On Sat, Nov 19, 2011 at 03:19:07PM +0100, Ingo Schwarze wrote:
>>> joerg@mdocml.bsd.lv wrote on Fri, Nov 18, 2011 at 11:39:08AM -0500:

>>>> Log Message:
>>>> -----------
>>>> Convert an assert into an explicit check. man_unscope can be triggered
>>>> on unknown macros.
>>>> 
>>>> Modified Files:
>>>> --------------
>>>>     mdocml:
>>>>         man_macro.c

>>> Sorry, so far i failed to reproduce the problem.
>>> Can you provide an example?

>> .ab help2man is required to generate this page
>> 
>> was the case that originally triggered this.

> I think this also triggers the other case with missing default settings
> in man_term.c later.

Oh, now i see.
It's not mandoc(1) that is crashing on that one, but groff(1);
the .ab request means "abort".
Sure, .ab is ignored by mandoc, but when you run a unit test
with both groff and mandoc on a file containing .ab, the test
fails because groff fails.
Obviously, you have mistaken that for a mandoc failure.

So please back out both bogus patches, unless you can demonstrate
that there is a problem with mandoc.
The following two test files both work for me with mandoc
and both abort with groff.
Commenting out any number of .TH arguments, or the whole .TH line,
doesn't reveal any problems either.

Yours,
  Ingo


ischwarze@isnote $ cat badbefore.in     
.ab help2man is required to generate this page
.TH TH-BADBEFORE 1 "November 19, 2011"
.SH NAME
TH-badbefore \- unknown macro before .TH
.SH DESCRIPTION
Some text.
ischwarze@isnote $ cat badafter.in                                             
.TH TH-BADAFTER 1 "November 19, 2011"
.ab help2man is required to generate this page
.SH NAME
TH-badafter \- unknown macro after .TH
.SH DESCRIPTION
Some text.
ischwarze@isnote $ mandoc badbefore.in > /dev/null && echo ok                  
ok
ischwarze@isnote $ mandoc badafter.in > /dev/null && echo ok  
ok
ischwarze@isnote $ /usr/local/bin/nroff -mandoc -Tascii -c badbefore.in        
help2man is required to generate this page
ischwarze@isnote $ /usr/local/bin/nroff -mandoc -Tascii -c badafter.in  
TH-BADAFTER(1)                                                  TH-BADAFTER(1)
help2man is required to generate this page
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Convert an assert into an explicit check.

[Joerg Sonnenberger]

On Sun, Nov 20, 2011 at 01:27:10PM +0100, Ingo Schwarze wrote:
> It's not mandoc(1) that is crashing on that one, but groff(1);

mandoc(1) was was crashing on this. I gave the complete input file.

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

Re: mdocml: Convert an assert into an explicit check.

[Ingo Schwarze]

Hi Joerg,

Joerg Sonnenberger wrote on Sun, Nov 20, 2011 at 08:17:18PM +0100:
> On Sun, Nov 20, 2011 at 01:27:10PM +0100, Ingo Schwarze wrote:

>> It's not mandoc(1) that is crashing on that one, but groff(1);

> mandoc(1) was was crashing on this. I gave the complete input file.

Ah, with that bit of information, i was able to reproduce and
finally came round to preparing a proper fix; thanks for the report.
I'm giving a patch against bsd.lv below; in OpenBSD, all that is needed
is removing the "assert(MAN_ROOT != m->last->type);" from function
man_unscope, file man_macro.c.

That function just works fine even when the syntax tree is completely
empty, only containing the ROOT node.  It is important to *NOT*
return early, but to continue through the whole function and execute
the man_valid_post(m) near the end - otherwise, you will end up
with NULL pointers in the meta structure and provoke the crashes
you observed.

OK?
  Ingo


Index: man_html.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man_html.c,v
retrieving revision 1.84
diff -u -p -r1.84 man_html.c
--- man_html.c	18 Nov 2011 17:05:50 -0000	1.84
+++ man_html.c	3 Dec 2011 21:37:58 -0000
@@ -305,8 +305,7 @@ man_root_pre(MAN_ARGS)
 	if (m->vol)
 		(void)strlcat(b, m->vol, BUFSIZ);
 
-	snprintf(title, BUFSIZ - 1, "%s(%s)", m->title ? m->title : "",
-	    m->msec ? m->msec : "");
+	snprintf(title, BUFSIZ - 1, "%s(%s)", m->title, m->msec);
 
 	PAIR_SUMMARY_INIT(&tag[0], "Document Header");
 	PAIR_CLASS_INIT(&tag[1], "head");
@@ -360,8 +359,7 @@ man_root_post(MAN_ARGS)
 	PAIR_CLASS_INIT(&tag[0], "foot-date");
 	print_otag(h, TAG_TD, 1, tag);
 
-	if (m->date)
-		print_text(h, m->date);
+	print_text(h, m->date);
 	print_stagq(h, tt);
 
 	PAIR_CLASS_INIT(&tag[0], "foot-os");
Index: man_macro.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man_macro.c,v
retrieving revision 1.69
diff -u -p -r1.69 man_macro.c
--- man_macro.c	18 Nov 2011 17:06:19 -0000	1.69
+++ man_macro.c	3 Dec 2011 21:37:58 -0000
@@ -120,8 +120,6 @@ man_unscope(struct man *m, const struct 
 
 	assert(to);
 
-	if (MAN_ROOT == m->last->type)
-		return(1);
 	m->next = MAN_NEXT_SIBLING;
 
 	/* LINTED */
Index: man_term.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man_term.c,v
retrieving revision 1.124
diff -u -p -r1.124 man_term.c
--- man_term.c	18 Nov 2011 17:04:06 -0000	1.124
+++ man_term.c	3 Dec 2011 21:37:58 -0000
@@ -969,9 +969,8 @@ print_man_foot(struct termp *p, const vo
 	term_vspace(p);
 	term_vspace(p);
 	term_vspace(p);
-	snprintf(title, BUFSIZ, "%s(%s)", meta->title ? meta->title : "",
-	    meta->msec ? meta->msec : "");
-	datelen = term_strlen(p, meta->date ? meta->date : "");
+	snprintf(title, BUFSIZ, "%s(%s)", meta->title, meta->msec);
+	datelen = term_strlen(p, meta->date);
 
 	p->flags |= TERMP_NOSPACE | TERMP_NOBREAK;
 	p->offset = 0;
@@ -987,7 +986,7 @@ print_man_foot(struct termp *p, const vo
 	if (p->offset + datelen >= p->rmargin)
 		p->rmargin = p->offset + datelen;
 
-	term_word(p, meta->date ? meta->date : "");
+	term_word(p, meta->date);
 	term_flushln(p);
 
 	p->flags &= ~TERMP_NOBREAK;
@@ -1024,8 +1023,7 @@ print_man_head(struct termp *p, const vo
 		strlcpy(buf, m->vol, BUFSIZ);
 	buflen = term_strlen(p, buf);
 
-	snprintf(title, BUFSIZ, "%s(%s)", m->title ? m->title : "",
-	    m->msec ? m->msec : "");
+	snprintf(title, BUFSIZ, "%s(%s)", m->title, m->msec);
 	titlen = term_strlen(p, title);
 
 	p->flags |= TERMP_NOBREAK | TERMP_NOSPACE;
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Convert an assert into an explicit check.

[Joerg Sonnenberger]

On Sat, Dec 03, 2011 at 10:41:57PM +0100, Ingo Schwarze wrote:
> That function just works fine even when the syntax tree is completely
> empty, only containing the ROOT node.  It is important to *NOT*
> return early, but to continue through the whole function and execute
> the man_valid_post(m) near the end - otherwise, you will end up
> with NULL pointers in the meta structure and provoke the crashes
> you observed.

In that case at least assert that m->title etc are not NULL.

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

Re: mdocml: Convert an assert into an explicit check.

[Ingo Schwarze]

Hi Joerg,

Joerg Sonnenberger wrote on Sat, Dec 03, 2011 at 11:47:55PM +0100:
> On Sat, Dec 03, 2011 at 10:41:57PM +0100, Ingo Schwarze wrote:

>> That function just works fine even when the syntax tree is completely
>> empty, only containing the ROOT node.  It is important to *NOT*
>> return early, but to continue through the whole function and execute
>> the man_valid_post(m) near the end - otherwise, you will end up
>> with NULL pointers in the meta structure and provoke the crashes
>> you observed.

> In that case at least assert that m->title etc are not NULL.

Sure, that's not going to do harm.

OK?
  Ingo


Index: man_html.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man_html.c,v
retrieving revision 1.84
diff -u -p -r1.84 man_html.c
--- man_html.c	18 Nov 2011 17:05:50 -0000	1.84
+++ man_html.c	3 Dec 2011 23:47:24 -0000
@@ -178,6 +178,8 @@ print_man_head(MAN_ARGS)
 {
 
 	print_gen_head(h);
+	assert(m->title);
+	assert(m->msec);
 	bufcat_fmt(h, "%s(%s)", m->title, m->msec);
 	print_otag(h, TAG_TITLE, 0, NULL);
 	print_text(h, h->buf);
@@ -305,8 +307,9 @@ man_root_pre(MAN_ARGS)
 	if (m->vol)
 		(void)strlcat(b, m->vol, BUFSIZ);
 
-	snprintf(title, BUFSIZ - 1, "%s(%s)", m->title ? m->title : "",
-	    m->msec ? m->msec : "");
+	assert(m->title);
+	assert(m->msec);
+	snprintf(title, BUFSIZ - 1, "%s(%s)", m->title, m->msec);
 
 	PAIR_SUMMARY_INIT(&tag[0], "Document Header");
 	PAIR_CLASS_INIT(&tag[1], "head");
@@ -360,8 +363,8 @@ man_root_post(MAN_ARGS)
 	PAIR_CLASS_INIT(&tag[0], "foot-date");
 	print_otag(h, TAG_TD, 1, tag);
 
-	if (m->date)
-		print_text(h, m->date);
+	assert(m->date);
+	print_text(h, m->date);
 	print_stagq(h, tt);
 
 	PAIR_CLASS_INIT(&tag[0], "foot-os");
Index: man_macro.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man_macro.c,v
retrieving revision 1.69
diff -u -p -r1.69 man_macro.c
--- man_macro.c	18 Nov 2011 17:06:19 -0000	1.69
+++ man_macro.c	3 Dec 2011 23:47:24 -0000
@@ -120,8 +120,6 @@ man_unscope(struct man *m, const struct 
 
 	assert(to);
 
-	if (MAN_ROOT == m->last->type)
-		return(1);
 	m->next = MAN_NEXT_SIBLING;
 
 	/* LINTED */
Index: man_term.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/man_term.c,v
retrieving revision 1.124
diff -u -p -r1.124 man_term.c
--- man_term.c	18 Nov 2011 17:04:06 -0000	1.124
+++ man_term.c	3 Dec 2011 23:47:24 -0000
@@ -963,15 +963,17 @@ print_man_foot(struct termp *p, const vo
 	const struct man_meta *meta;
 
 	meta = (const struct man_meta *)arg;
+	assert(meta->title);
+	assert(meta->msec);
+	assert(meta->date);
 
 	term_fontrepl(p, TERMFONT_NONE);
 
 	term_vspace(p);
 	term_vspace(p);
 	term_vspace(p);
-	snprintf(title, BUFSIZ, "%s(%s)", meta->title ? meta->title : "",
-	    meta->msec ? meta->msec : "");
-	datelen = term_strlen(p, meta->date ? meta->date : "");
+	snprintf(title, BUFSIZ, "%s(%s)", meta->title, meta->msec);
+	datelen = term_strlen(p, meta->date);
 
 	p->flags |= TERMP_NOSPACE | TERMP_NOBREAK;
 	p->offset = 0;
@@ -987,7 +989,7 @@ print_man_foot(struct termp *p, const vo
 	if (p->offset + datelen >= p->rmargin)
 		p->rmargin = p->offset + datelen;
 
-	term_word(p, meta->date ? meta->date : "");
+	term_word(p, meta->date);
 	term_flushln(p);
 
 	p->flags &= ~TERMP_NOBREAK;
@@ -1008,6 +1010,8 @@ print_man_head(struct termp *p, const vo
 	const struct man_meta *m;
 
 	m = (const struct man_meta *)arg;
+	assert(m->title);
+	assert(m->msec);
 
 	/*
 	 * Note that old groff would spit out some spaces before the
@@ -1024,8 +1028,7 @@ print_man_head(struct termp *p, const vo
 		strlcpy(buf, m->vol, BUFSIZ);
 	buflen = term_strlen(p, buf);
 
-	snprintf(title, BUFSIZ, "%s(%s)", m->title ? m->title : "",
-	    m->msec ? m->msec : "");
+	snprintf(title, BUFSIZ, "%s(%s)", m->title, m->msec);
 	titlen = term_strlen(p, title);
 
 	p->flags |= TERMP_NOBREAK | TERMP_NOSPACE;
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Convert an assert into an explicit check.

[Joerg Sonnenberger]

On Sun, Dec 04, 2011 at 12:50:05AM +0100, Ingo Schwarze wrote:
> Hi Joerg,
> 
> Joerg Sonnenberger wrote on Sat, Dec 03, 2011 at 11:47:55PM +0100:
> > On Sat, Dec 03, 2011 at 10:41:57PM +0100, Ingo Schwarze wrote:
> 
> >> That function just works fine even when the syntax tree is completely
> >> empty, only containing the ROOT node.  It is important to *NOT*
> >> return early, but to continue through the whole function and execute
> >> the man_valid_post(m) near the end - otherwise, you will end up
> >> with NULL pointers in the meta structure and provoke the crashes
> >> you observed.
> 
> > In that case at least assert that m->title etc are not NULL.
> 
> Sure, that's not going to do harm.
> 
> OK?

OK.

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