From owner-svn-src-all@freebsd.org Sat Jul 30 00:00:33 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6F374BA8527; Sat, 30 Jul 2016 00:00:33 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id 080AF1301; Sat, 30 Jul 2016 00:00:32 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c122-106-149-109.carlnfd1.nsw.optusnet.com.au (c122-106-149-109.carlnfd1.nsw.optusnet.com.au [122.106.149.109]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id 647313C1BEE; Sat, 30 Jul 2016 10:00:05 +1000 (AEST) Date: Sat, 30 Jul 2016 10:00:05 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Pedro F. Giffuni" cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r303502 - head/usr.bin/indent In-Reply-To: <201607291936.u6TJaALj035130@repo.freebsd.org> Message-ID: <20160730092525.N1012@besplex.bde.org> References: <201607291936.u6TJaALj035130@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=OtmysHLt c=1 sm=1 tr=0 a=R/f3m204ZbWUO/0rwPSMPw==:117 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=kj9zAlcOel0A:10 a=V1RT_qUmue8vlDhu7tIA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 00:00:33 -0000 On Fri, 29 Jul 2016, Pedro F. Giffuni wrote: > Log: > indent(1): Use NULL instead of zero for pointers. This is probably not indent's style, I doubt that you found all of the implicit NULLs. A recent commit added strstr() without even a comparison with 0. This adds some lexical style bugs. > Modified: head/usr.bin/indent/indent.c > ============================================================================== > --- head/usr.bin/indent/indent.c Fri Jul 29 18:26:15 2016 (r303501) > +++ head/usr.bin/indent/indent.c Fri Jul 29 19:36:10 2016 (r303502) > @@ -341,8 +341,8 @@ main(int argc, char **argv) > } > case comment: /* we have a comment, so we must copy it into > * the buffer */ > - if (!flushed_nl || sc_end != 0) { > - if (sc_end == 0) { /* if this is the first comment, we > + if (!flushed_nl || sc_end != NULL) { > + if (sc_end == NULL) { /* if this is the first comment, we > * must set up the buffer */ > save_com[0] = save_com[1] = ' '; > sc_end = &(save_com[2]); This breaks the formatting using blind substitution. A program named indent should be used to check the formatting. indent -l is mostly broken, but it works for comments, and indent's source code needs it to format comments more than most programs because indent puts lots of comments to the right of the code where they get moved further right by expansions. Normally, reformatting of such comments is not wanted, but it should be used for new code. I added the -[n]fcb to disable excessive reformatting of comments to the right of code. This only applies to block comments. > @@ -1101,9 +1101,9 @@ check_type: > > while (e_lab > s_lab && (e_lab[-1] == ' ' || e_lab[-1] == '\t')) > e_lab--; > - if (e_lab - s_lab == com_end && bp_save == 0) { /* comment on > - * preprocessor line */ This was a good example of indent's bad style. The comment needed to be in coulumns 64-78 to follow the style. After expansion, it needs to be in columns 72-78. But 2 of the 3 words in it are too long to fit there. > - if (sc_end == 0) /* if this is the first comment, we > + /* comment on preprocessor line */ The expansion wasn't blind, and the comment was moved to fit on new line. indent would have problems moving it, but it wouldn't move it to a wrong place -- from inside the compound statement to outside. > + if (e_lab - s_lab == com_end && bp_save == NULL) { > + if (sc_end == NULL) /* if this is the first comment, we > * must set up the buffer */ > sc_end = &(save_com[0]); > else { indent would have kept it inside the compound statement. It can then be started at the same indentation level as the code, or at the right of a blank statement. The comment could be corrected to live outside of the compound statement by adding an "if" condition to it. That is too hard for indent. The first coment inside the comment gives an example of using an "if" condition but this is not quite right here -- "if" only applies before the test. > Modified: head/usr.bin/indent/io.c > ============================================================================== > --- head/usr.bin/indent/io.c Fri Jul 29 18:26:15 2016 (r303501) > +++ head/usr.bin/indent/io.c Fri Jul 29 19:36:10 2016 (r303502) > @@ -348,10 +348,10 @@ fill_buffer(void) > int i; > FILE *f = input; > > - if (bp_save != 0) { /* there is a partly filled input buffer left */ > + if (bp_save != NULL) { /* there is a partly filled input buffer left */ The formatting was already broken here. It is especially broken since nearby lines use a smaller indentation that would work here too. > buf_ptr = bp_save; /* dont read anything, just switch buffers */ indent dont need know spelling lessons? > buf_end = be_save; > - bp_save = be_save = 0; > + bp_save = be_save = NULL; > if (buf_ptr < buf_end) > return; /* only return if there is really something in > * this buffer */ Bruce