From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp1.rz.uni-karlsruhe.de (Debian-exim@smtp1.rz.uni-karlsruhe.de [129.13.185.217]) by krisdoz.my.domain (8.14.3/8.14.3) with ESMTP id o91Mbio4017449 for ; Fri, 1 Oct 2010 18:37:44 -0400 (EDT) Received: from hekate.usta.de (asta-nat.asta.uni-karlsruhe.de [172.22.63.82]) by smtp1.rz.uni-karlsruhe.de with esmtp (Exim 4.63 #1) id 1P1oEY-0006jw-UU; Sat, 02 Oct 2010 00:37:43 +0200 Received: from donnerwolke.usta.de ([172.24.96.3]) by hekate.usta.de with esmtp (Exim 4.71) (envelope-from ) id 1P1oEY-0001oK-TS for tech@mdocml.bsd.lv; Sat, 02 Oct 2010 00:37:42 +0200 Received: from iris.usta.de ([172.24.96.5] helo=usta.de) by donnerwolke.usta.de with esmtp (Exim 4.69) (envelope-from ) id 1P1oEY-0006Pn-SU for tech@mdocml.bsd.lv; Sat, 02 Oct 2010 00:37:42 +0200 Received: from schwarze by usta.de with local (Exim 4.71) (envelope-from ) id 1P1oEY-0001Wg-Rq for tech@mdocml.bsd.lv; Sat, 02 Oct 2010 00:37:42 +0200 Date: Sat, 2 Oct 2010 00:37:42 +0200 From: Ingo Schwarze To: tech@mdocml.bsd.lv Subject: Re: term.c and lint Message-ID: <20101001223742.GD17696@iris.usta.de> References: <4C9F1241.9040706@hhs.se> <20100926172709.GC3502@iris.usta.de> <4CA061EC.1070006@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: <4CA061EC.1070006@bsd.lv> User-Agent: Mutt/1.5.20 (2009-06-14) Hi Kristaps, Kristaps Dzonsons wrote on Mon, Sep 27, 2010 at 11:20:44AM +0200: > I'm completely with you on this one. OpenBSD's lint is overzealous, > especially when indexing with size_t. It's ridiculous: malloc and > company /demand/ size_t when allocating memory---yet the same type > can't be used to iterate over the array? Faugh. > > Please leave the code as-is, in this case: I just wanted to be sure > that they warnings are benign. Hm, i think part of these changes actually make the code easier to understand. I mostly freaked out on those that don't seem to allow any sane way to please lint, and of course on the indexing issue. > I'll probably slowly cast out (har, har) explicit casts used to quiesce > lint. The same goes with those absurd pre-loop LINTED comments. Yes, i think getting rid of most of the LINTED comments is good, because they don't even tell you what is special about the next line. I also think that explicit casts when indexing arrays is rather ugly. But when you do indeed cast from one type to another, not trivially compatible type, making the cast explicit does not seem that bad, it provides a hint that something potentially dangerous is happening at that place. As long as we have many such casts in our code, i'm not sure having them implicit and lint complain is better than having them explicit. Rather, i'm wondering whether there are techniques to avoid such problems in the first place, in the design stage or whenever... That said, i have thrown out those parts of my term.c diff that seemed silly to me. I think the rest makes sense in any case. As it is tested, i could as well put it in, right? This is what it does: * make the initial maxvis/mmax calculation easier to understand * where real, non-indexing casts happen, make them explicit * avoid a few lint warnings that can easily be fixed * remove one needless LINTED comment Yours, Ingo Index: term.c =================================================================== RCS file: /cvs/src/usr.bin/mandoc/term.c,v retrieving revision 1.52 diff -u -p -r1.52 term.c --- term.c 1 Oct 2010 21:38:26 -0000 1.52 +++ term.c 1 Oct 2010 21:59:48 -0000 @@ -142,24 +142,21 @@ term_flushln(struct termp *p) * an indentation, but can be, for tagged lists or columns, a * small set of values. */ - - assert(p->offset < p->rmargin); - - maxvis = (int)(p->rmargin - p->offset) - p->overstep < 0 ? - /* LINTED */ - 0 : p->rmargin - p->offset - p->overstep; - mmax = (int)(p->maxrmargin - p->offset) - p->overstep < 0 ? - /* LINTED */ - 0 : p->maxrmargin - p->offset - p->overstep; + assert (p->rmargin > p->offset); + dv = p->rmargin - p->offset; + maxvis = (int)dv > p->overstep ? dv - (size_t)p->overstep : 0; + dv = p->maxrmargin - p->offset; + mmax = (int)dv > p->overstep ? dv - (size_t)p->overstep : 0; bp = TERMP_NOBREAK & p->flags ? mmax : maxvis; /* * Indent the first line of a paragraph. */ - vbl = p->flags & TERMP_NOLPAD ? 0 : p->offset; + vbl = p->flags & TERMP_NOLPAD ? (size_t)0 : p->offset; - vis = vend = i = 0; + vis = vend = 0; + i = 0; while (i < (int)p->col) { /* @@ -180,7 +177,6 @@ term_flushln(struct termp *p) * space is printed according to regular spacing rules). */ - /* LINTED */ for (j = i, jhy = 0; j < (int)p->col; j++) { if ((j && ' ' == p->buf[j]) || '\t' == p->buf[j]) break; @@ -219,8 +215,7 @@ term_flushln(struct termp *p) /* Remove the p->overstep width. */ - bp += (int)/* LINTED */ - p->overstep; + bp += (size_t)p->overstep; p->overstep = 0; } @@ -234,7 +229,7 @@ term_flushln(struct termp *p) j = i; while (' ' == p->buf[i]) i++; - dv = (i - j) * (*p->width)(p, ' '); + dv = (size_t)(i - j) * (*p->width)(p, ' '); vbl += dv; vend += dv; break; @@ -283,8 +278,7 @@ term_flushln(struct termp *p) if (TERMP_HANG & p->flags) { /* We need one blank after the tag. */ - p->overstep = /* LINTED */ - vis - maxvis + (*p->width)(p, ' '); + p->overstep = (int)(vis - maxvis + (*p->width)(p, ' ')); /* * Behave exactly the same way as groff: @@ -298,8 +292,7 @@ term_flushln(struct termp *p) */ if (p->overstep >= -1) { assert((int)maxvis + p->overstep >= 0); - /* LINTED */ - maxvis += p->overstep; + maxvis += (size_t)p->overstep; } else p->overstep = 0; @@ -307,9 +300,8 @@ term_flushln(struct termp *p) return; /* Right-pad. */ - if (maxvis > vis + /* LINTED */ - ((TERMP_TWOSPACE & p->flags) ? - (*p->width)(p, ' ') : 0)) { + if (maxvis > vis + + ((TERMP_TWOSPACE & p->flags) ? (*p->width)(p, ' ') : 0)) { p->viscol += maxvis - vis; (*p->advance)(p, maxvis - vis); vis += (maxvis - vis); -- To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv