bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] test-exclude.c: remove unmatched #endif


From: Bruno Haible
Subject: Re: [PATCH] test-exclude.c: remove unmatched #endif
Date: Sun, 20 Feb 2011 23:38:57 +0100
User-agent: KMail/1.9.9

Hi Jim,

> I was surprised to find that a test did not compile.
> * tests/test-exclude.c: Remove stray #endif, left over from
> the change of a week ago.

Thanks for the fix; my mistake.

> This one was interesting in that it arose (I suspect)
> due to the fact that code upon which the offending change
> was based had misleadingly-indented cpp directives.

Yes, this was part of the problem. The other part was that blank lines
normally help to find out the start and end of semantic blocks; in this
case, the absence of blank lines misled me.

> Easiest would be to convert
> all files, regardless of owner; would anyone prefer I *not* do that?

Yes, I am opposed to it. No way to have me agree to changing more than
50 files.

10 files would be ok. In other words, the proposed change is too far
away from current practice. I could agree to something that is closer
to current practice, yet avoids some of the biggest pitfalls.

The pitfall in this case was to have a block of lines, without blank lines
in them, which had an unbalanced #if[def]/#endif count _and_ more than
one #if[def]/#endif _and_ was not using cppi style.

This is OK for me (does not present a pitfall):

  #if A
  #if B
  foo
  #endif
  #endif

This is OK too:

  #if A
  # if B

  # endif

  #endif

This is OK too:

  #if A

  #if B

  #endif

  #endif

This is OK too:

  #if A

  #if B
  foo
  #endif

  #endif

But this is not OK:

  #if A
  #if B
  foo
  #endif

  #endif

Can cppi be extended (through a command-line option) to flag only the last case 
as
improperly indented? I haven't found a way to do so within 10 minutes, so I 
wrote a
tool from scratch for doing this. It has some limitations / bugs (*), but on 
the gnulib
source code before the 2011-02-13 change it produces these warnings:

  $ for f in `find lib -name '*.[hc]' | sort` `find tests -name '*.[hc]' | 
sort`; do ./findmisindent $f; done
  misindented block at lib/argp-namefrob.h:98
  misindented block at lib/getopt_int.h:108
  misindented block at lib/progreloc.c:144
  misindented block at lib/progreloc.c:256
  misindented block at lib/regexec.c:3971
  misindented block at lib/vasnprintf.c:879
  misindented block at lib/vasnprintf.c:4568
  misindented block at tests/test-argmatch.c:29
  misindented block at tests/test-exclude.c:63

I.e. to fix this, we need to touch less than 10 files, and it catches to pitfall
in tests/test-exclude.c.

(*) It does not have --help. It does not do argument checking. It uses gets(). 
It does
    not know about comment syntax. It does not know about preprocessor directive
    continuation lines.

Bruno
-- 
In memoriam Juliusz Bursche <http://en.wikipedia.org/wiki/Juliusz_Bursche>

Attachment: findmisindent.c
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]