[GRASS-dev] GRASS 7 development started

[Glynn Clements]

Markus Neteler wrote:

> >> do you have the ultimate "indent" line for me?
> >
> > indent -bad -bap -bbb -br -bli0 -bls -cli0 -ncs -fc1 -hnl -i4 \
> >       -nbbo -nbc -nbfda -nbfde -ncdb -ncdw -nce -nfca -npcs -nprs \
> >       -npsl -nsc -nsob -saf -sai -saw -sbi0 -ss -ts8 -ut
> 
> I have done a test run locally and came across a problem:
> 
> cd visualization/nviz/src
>  indent -bad -bap -bbb -br -bli0 -bls -cli0 -ncs -fc1 -hnl -i4 \
>        -nbbo -nbc -nbfda -nbfde -ncdb -ncdw -nce -nfca -npcs -nprs \
>        -npsl -nsc -nsob -saf -sai -saw -sbi0 -ss -ts8 -ut togl_flythrough.c
> 
> indent: togl_flythrough.c:609: Warning:old style assignment ambiguity
> in "=-".  Assuming "= -"

These warnings are harmless. K&R C allowed "=-", "=+" etc as aliases
for "-=", "+=", etc. But I have never even heard of anyone
encountering this syntax in actual code, and certainly haven't seen it
myself. Also, it only works with K&R-style compilers; gcc doesn't even
have an option to support it.

> indent: togl_flythrough.c:956: Error:Unexpected end of file

This is caused by the "README" in the "#if 0 ..." block at the top of
the file. If you remove that block, the error goes away.

Even code which is disabled with "#if 0 ..." should at least parse as
C code. Otherwise, it tends to mess up formatting and syntax
highlighting in text editors (which usually just ignore preprocessor
directives).

> Additionally, there are:
> 
> indent: ./lib/vector/dglib/misc-template.c:546: Error:Stmt nesting error.
> indent: ./lib/vector/dglib/misc-template.c:623: Error:Unmatched 'else'
> indent: ./lib/vector/dglib/misc-template.c:649: Error:Stmt nesting error.

That's caused by putting unmatched braces inside a #if. Again, this
tends not to work with tools which operate upon source code.

Note that misc-template.c isn't actually used.

> indent: ./swig/perl/grass_wrap.c:1064: Error:Stmt nesting error.

Again, unmatched braces inside #if. But:

1. That file shouldn't even be under version control, as it isn't a
source file (it's an intermediate file, generated by SWIG).
2. It shouldn't be indent-ed (or modified in any way, as it's
auto-generated).
3. It should arguably have a .cc extension as it contains C++ code.

> and several warnings like
> indent: ./lib/sites/sites.c:859: Warning:old style assignment
> ambiguity in "=-".  Assuming "= -"
> and
> indent: ./lib/rst/interp_float/vinput2d.c:75: Warning:old style
> assignment ambiguity in "=*".  Assuming "= *"

Harmless, see above.

> Analysing one of the 'Assuming "= -"' warnings, it turns out that:
>                     site->cattype=-1;
> is changed to
>                     site->cattype = -1;
> but it should be
>                     site->cattype =- 1;

No, it should be "site->cattype = -1;", as it's assigning negative
one, not decrementing.

 This is sites/s.in.ascii/get_site.c:130, right? I'm quite
certain that it really should be assigning -1, not decrementing.

If was really supposed to be a decrement operation, it would need to
be either:

	site->cattype -= 1;
or:
	site->cattype--;

in order to work with existing compilers.

> Can we trap this? Otherwise we can run it locally and fix all warnings
> manually in SVN before running the global automated indenting.

You can verify that the indentation isn't changing the semantics of
the code by compiling everything[1] without -g and with -DNDEBUG,
before and after indenting, and comparing the .o files.

[1] If you want to be 100% certain, you will need use all of the
--with-<package> switches (which means installing all optional
dependencies), and on multiple platforms.

When I tried it on Linux with everything except MySQL, SQLite and
OpenDWG, I didn't find any mismatches. That probably accounts for 99%
of the code, so the chances are slim that there are any problems in
the fraction of the code which didn't get compiled.

-- 
Glynn Clements <glynn at gclements.plus.com>