Date: Sat, 30 Jul 2016 10:00:05 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "Pedro F. Giffuni" <pfg@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r303502 - head/usr.bin/indent Message-ID: <20160730092525.N1012@besplex.bde.org> In-Reply-To: <201607291936.u6TJaALj035130@repo.freebsd.org> References: <201607291936.u6TJaALj035130@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160730092525.N1012>