From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailout.scc.kit.edu (mailout.scc.kit.edu [129.13.185.202]) by krisdoz.my.domain (8.14.5/8.14.5) with ESMTP id s7I5oT9e032750 for ; Mon, 18 Aug 2014 01:50:32 -0400 (EDT) Received: from hekate.usta.de (asta-nat.asta.uni-karlsruhe.de [172.22.63.82]) by scc-mailout-02.scc.kit.edu with esmtp (Exim 4.72 #1) id 1XJFps-0006Fz-GO; Mon, 18 Aug 2014 07:50:28 +0200 Received: from donnerwolke.usta.de ([172.24.96.3]) by hekate.usta.de with esmtp (Exim 4.77) (envelope-from ) id 1XJFpo-0001cI-F3; Mon, 18 Aug 2014 07:50:24 +0200 Received: from iris.usta.de ([172.24.96.5] helo=usta.de) by donnerwolke.usta.de with esmtp (Exim 4.72) (envelope-from ) id 1XJFpo-0002Op-Aw; Mon, 18 Aug 2014 07:50:24 +0200 Received: from schwarze by usta.de with local (Exim 4.77) (envelope-from ) id 1XJFp3-0005Cj-Va; Mon, 18 Aug 2014 07:49:37 +0200 Date: Mon, 18 Aug 2014 07:49:37 +0200 From: Ingo Schwarze To: Kristaps Dzonsons Cc: tech@mdocml.bsd.lv Subject: Re: Valgrind Error Message-ID: <20140818054937.GA21733@iris.usta.de> References: <53F1451A.5080106@bsd.lv> X-Mailinglist: mdocml-tech Reply-To: tech@mdocml.bsd.lv MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53F1451A.5080106@bsd.lv> User-Agent: Mutt/1.5.21 (2010-09-15) Hi Kristaps, Kristaps Dzonsons wrote on Mon, Aug 18, 2014 at 02:13:14AM +0200: > So to test HEAD (accidentally--I was supposed to be testing > 1.12.4-rc1 but ran from the wrong directory), Additional testing is always welcome. :) > I wrote a little > script to run through all the manuals on my system with mandoc via > valgrind. It has only found one nit on ALL of them. On a binary > file, at that! [...] > I tracked this down to passing "\H" to mandoc_escape(). It > seems we're not very careful in this function to receiving \0 after > the initial marker, so enclosed are some check. I may have missed > some, so please pass a critical eye over this as well! > > Thoughts? The middle chunk for \h and friends seems correct. Indeed, *end must not advance past the \0. The first (for \f and friends) and third (for \s) seem redundant, though. Assume we enter with "\f\0". By the time we enter the case, *start and *end point to the \0. So the inner switch goes to the default, sets *sz to 1, and goes out of the end of the big switch with *start and *end both still pointing to the \0, which is legitimate. It's the job of the code after the switch to catch premature end of the argument. The big switch only catches the errors related to parsing the argument delimiters, not the argument itself. Argument parsing after the switch seems correct. If term is set, we go into the while, where - first thing - the switch catches \0. Otherwise, *sz is compated to strlen(*start), in this case 1 > 0, so we return right away, too. I'd say, please commit the middle chunk. The other two don't add clarity, they merely muddle the point which code is responsible for parsing what. Reading the code, i didn't find any other accesses past a \0. The dangerous cases (having argument delimiters) are \C, \A, and \N (which already check) and \h (which you correct). So it looks like an isolated bug in the one case you found. Yours, Ingo > Index: mandoc.c > =================================================================== > RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mandoc.c,v > retrieving revision 1.85 > diff -u -p -r1.85 mandoc.c > --- mandoc.c 16 Aug 2014 19:00:01 -0000 1.85 > +++ mandoc.c 17 Aug 2014 23:56:04 -0000 > @@ -150,6 +150,8 @@ mandoc_escape(const char **end, const ch > *start = ++*end; > term = ']'; > break; > + case '\0': > + return(ESCAPE_ERROR); > default: > *sz = 1; > break; > @@ -199,7 +201,8 @@ mandoc_escape(const char **end, const ch > /* FALLTHROUGH */ > case 'x': > if (strchr(" %&()*+-./0123456789:<=>", **start)) { > - ++*end; > + if ('\0' != **start) > + ++*end; > return(ESCAPE_ERROR); > } > gly = ESCAPE_IGNORE; > @@ -250,6 +253,8 @@ mandoc_escape(const char **end, const ch > *start = ++*end; > term = '\''; > break; > + case '\0': > + return(ESCAPE_ERROR); > default: > *sz = 1; > break; -- To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv