From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-1.sys.kth.se (smtp-1.sys.kth.se [130.237.32.175]) by krisdoz.my.domain (8.14.3/8.14.3) with ESMTP id p02LZfYU006935 for ; Sun, 2 Jan 2011 16:35:42 -0500 (EST) Received: from smtp-1.sys.kth.se (localhost [127.0.0.1]) by smtp-1.sys.kth.se (Postfix) with ESMTP id 0434C156384 for ; Sun, 2 Jan 2011 22:35:36 +0100 (CET) X-Virus-Scanned: by amavisd-new at kth.se Received: from smtp-1.sys.kth.se ([127.0.0.1]) by smtp-1.sys.kth.se (smtp-1.sys.kth.se [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 3iKJl39SftJ2 for ; Sun, 2 Jan 2011 22:35:34 +0100 (CET) X-KTH-Auth: kristaps [85.8.61.101] X-KTH-mail-from: kristaps@bsd.lv X-KTH-rcpt-to: tech@mdocml.bsd.lv Received: from h85-8-61-101.dynamic.se.alltele.net (h85-8-61-101.dynamic.se.alltele.net [85.8.61.101]) by smtp-1.sys.kth.se (Postfix) with ESMTP id 8422E154129 for ; Sun, 2 Jan 2011 22:35:33 +0100 (CET) Message-ID: <4D20EFA4.6090409@bsd.lv> Date: Sun, 02 Jan 2011 22:35:32 +0100 From: Kristaps Dzonsons User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.13) Gecko/20101207 Thunderbird/3.1.7 X-Mailinglist: mdocml-tech Reply-To: tech@mdocml.bsd.lv MIME-Version: 1.0 To: tech@mdocml.bsd.lv Subject: Re: [PATCH] partial cleanup of mdoc(7) arg count validation References: <20110102211552.GB21085@iris.usta.de> In-Reply-To: <20110102211552.GB21085@iris.usta.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit > when i ran into a few segfaults, i had a look at argument count > validation in libmdoc, and it revealed itself as a can of worms. > > First, i did some framework improvements: > * Added MANDOCERR_ARGCWARN to support real warnings. > * Removed CHECK_FATAL because it was unused, and i can't see how > a wrong number of arguments on any macro line could ever force > us to abort the parser: For missing arguments, one can assume > defaults, and excessive ones can be discarded. > * Use the correct level in check_count(). > * Improve the wording: "more than N children", not "greater than". > > Then, the bug fixes that made me look at this: > * Don't segfault on empty .Rs. > * Don't segfault on empty .Sm and .Db; do the required checks in ebool(). > * Don't segfault on empty .St; do the required checks in post_st(). > > While there, i did lots of message level adjustments, all downgrades > from error to warnings, because none of these is likely to loose relevant > information. Some macros had their own validation function anyway, > so use these to do all checks, making the code easier to follow: > * .An -split with additional arguments > * .Lb with wrong number of arguments (all checks in post_lb) > * .Rs with header arguments > * .Rs with an empty body (all checks in post_rs) > * posts_text1: These three macros (.In, .Pf, .%U) have more problems: > Empty .In and .%U should be removed, and the first word after .Pf > should not be parsed, but i don't touch that now. > * .sp with excessive arguments > > The macros .Dl and .D1 don't need their HEAD children check: > These macros are blk_part_imp, so they cannot have HEAD children; > we could make it an assertion, but they have no dedicated validation > functions, so i guess it's not worth the effort. > > After doing all this, i could emove the now unused functions > eerr_eq0, eerr_eq1, eerr_le1, herr_eq0, herr_ge1, > only adding ewarn_eq1, ewarn_le1, hwarn_ge1. > > The bad news is that eerr_ge1 still needs to be checked. > Probably, it can be changed to ewarn_ge1; but after finding so many > issues with the other functions, i'm reluctant to do that blindly, > so i'm postponing it for now. It is only used by .Ad .Cd .Er .Fn > .Ic .%A .%B .%D .%I .%J .%N .%O .%P .%R .%T .%V .Ms .Sx .Sy .Tn .Lk > .%C .%Q .Vt, so that should be a quick check. ;-/ > > OK for what i have so far? Ingo, These look fine by me, as the ERROR->WARN downgrade doesn't effect a semantic difference. In general I encourage two functions instead of one (e.g., checking whether an argument exist (1) and checking whether it's bool (2)) to avoid code duplication. If your way doesn't bloat the code any, however, I'm not opposed to it. And by all means, feel free to remove those superfluous HEAD checks... Thanks, Kristaps -- To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv