[GRASS5] ANSIfication of GRASS 6.1-CVS functions

Hi developers,

I have submitting updated function declarations
to CVS to migrate old K&R style to ANSI style. About
800 functions in 230 files are affected.

This work was only possible thanks to the efforts
of Prof Giulio Antoniol (École Polytechnique, Montreal,
Canada) who analyzed the 6.1-CVS code and wrote a PERL
script for me/us to auto-convert the function
declarations.

I am submitting things only after inspection.
The list of changes (CVS diffs) is available at:
http://grass.itc.it/pipermail/grass-commit/2006-January/date.html#end

My gcc flags for compilation are (pretty restrictive):
  MYCFLAGS="-g -Wall -Werror-implicit-function-declaration -fno-common"
  MYCXXFLAGS="-g -Wall"
  CFLAGS="$MYCFLAGS" CXXFLAGS="$MYCXXFLAGS" ./configure ...

A few doubts I have which I would like to discuss here
before submitting:

####################
I fixed (hopefully correctly) the 'wext()' definition
and calls in

lib/cdhc/as181.c

####################
Then a conflict in lib/gis/:

definition in gisdefs.h:

/* index.c */
char *G_index(char *, int);
char *G_rindex(char *, int);

definition in index.c:

char *
G_index (register char *str, register char delim)

char *
G_rindex (register char *str, register char delim)

####################
another one in lib/gis

/* unctrl.c */
char *G_unctrl(int);

definition in unctrl.c
char *G_unctrl(unsigned char c);

####################
imagery/open.c

static error (char *group, char *file, char *msga, char *msgb)

The compiler complains about absent return type:
open.c:12: warning: return type defaults to `int'
open.c: In function `error':
open.c:17: warning: control reaches end of non-void function

####################
Unrelated, I found that by chance:

bartok:gis[6941.292] grep 'floor()\|ceil()' *
align_window.c: double floor(), ceil();
color_range.c: double floor(), ceil();

I guess that it is no good idea to have it
defined, right?

####################
I hope that nothing got damaged. In a few cases the
type had to be assumed (usually 'int'). I tried to inspect
all such cases.

So: Please update from CVS and recompile and use it
as usual. Please try out especially
- g3d related modules
- libgis related modules (ok, most modules)

Cheers

Markus

Markus Neteler wrote:

I have submitting updated function declarations
to CVS to migrate old K&R style to ANSI style. About
800 functions in 230 files are affected.

This work was only possible thanks to the efforts
of Prof Giulio Antoniol (École Polytechnique, Montreal,
Canada) who analyzed the 6.1-CVS code and wrote a PERL
script for me/us to auto-convert the function
declarations.

Is there a problem with the "protoize" utility (from gcc)?

I am submitting things only after inspection.
The list of changes (CVS diffs) is available at:
http://grass.itc.it/pipermail/grass-commit/2006-January/date.html#end

My gcc flags for compilation are (pretty restrictive):
  MYCFLAGS="-g -Wall -Werror-implicit-function-declaration -fno-common"
  MYCXXFLAGS="-g -Wall"
  CFLAGS="$MYCFLAGS" CXXFLAGS="$MYCXXFLAGS" ./configure ...

A few doubts I have which I would like to discuss here
before submitting:

####################
I fixed (hopefully correctly) the 'wext()' definition
and calls in

lib/cdhc/as181.c

Note that, for function arguments, there is no semantic difference
between "double x" and "double *x".

Personally, I always use the latter as what is being passed is a
pointer, but some people like to use the different syntaxes to
indicate whether the pointer points to a single value (*x) or to
multiple values (x).

####################
Then a conflict in lib/gis/:

definition in gisdefs.h:

/* index.c */
char *G_index(char *, int);
char *G_rindex(char *, int);

definition in index.c:

char *
G_index (register char *str, register char delim)

char *
G_rindex (register char *str, register char delim)

Using "register" in argument lists is meaningless. Whether or not a
register is used is determined by the calling convention.

Linux/x86/gcc uses the stack by default; you can force the use of a
register using the "regparm" attribute, but that's seldom a good idea.

For local variables, it's almost always preferable to let the compiler
figure out when to use a register (and most modern compilers will just
ignore "register" specifiers).

####################
another one in lib/gis

/* unctrl.c */
char *G_unctrl(int);

definition in unctrl.c
char *G_unctrl(unsigned char c);

Change the implementation to match the declaration in gisdefs.h. The
K&R version will expect an int to have been passed, as that's the
smallest integral type that K&R allows.

####################
imagery/open.c

static error (char *group, char *file, char *msga, char *msgb)

The compiler complains about absent return type:
open.c:12: warning: return type defaults to `int'
open.c: In function `error':
open.c:17: warning: control reaches end of non-void function

Change it to "static void error(...)".

####################
Unrelated, I found that by chance:

bartok:gis[6941.292] grep 'floor()\|ceil()' *
align_window.c: double floor(), ceil();
color_range.c: double floor(), ceil();

I guess that it is no good idea to have it
defined, right?

Use "#include <math.h>" to get the prototypes.

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

On Mon, Jan 02, 2006 at 05:35:40PM +0000, Glynn Clements wrote:

Markus Neteler wrote:

> I have submitting updated function declarations
> to CVS to migrate old K&R style to ANSI style. About
> 800 functions in 230 files are affected.
>
> This work was only possible thanks to the efforts
> of Prof Giulio Antoniol (École Polytechnique, Montreal,
> Canada) who analyzed the 6.1-CVS code and wrote a PERL
> script for me/us to auto-convert the function
> declarations.

Is there a problem with the "protoize" utility (from gcc)?

I tried a while but didn't get it working and didn't
find instructions online which I managed to understand.
So I asked Giulio Antoniol, and he helped out.

> I am submitting things only after inspection.
> The list of changes (CVS diffs) is available at:
> http://grass.itc.it/pipermail/grass-commit/2006-January/date.html#end
>
> My gcc flags for compilation are (pretty restrictive):
> MYCFLAGS="-g -Wall -Werror-implicit-function-declaration -fno-common"
> MYCXXFLAGS="-g -Wall"
> CFLAGS="$MYCFLAGS" CXXFLAGS="$MYCXXFLAGS" ./configure ...
>
> A few doubts I have which I would like to discuss here
> before submitting:
>
> ####################
> I fixed (hopefully correctly) the 'wext()' definition
> and calls in
>
> lib/cdhc/as181.c

Note that, for function arguments, there is no semantic difference
between "double x" and "double *x".

Personally, I always use the latter as what is being passed is a
pointer, but some people like to use the different syntaxes to
indicate whether the pointer points to a single value (*x) or to
multiple values (x).

So far we preserved what was there. It is debatable to
make another round to get this updated as well.

> ####################
> Then a conflict in lib/gis/:
>
> definition in gisdefs.h:
>
> /* index.c */
> char *G_index(char *, int);
> char *G_rindex(char *, int);
>
> definition in index.c:
>
> char *
> G_index (register char *str, register char delim)
>
> char *
> G_rindex (register char *str, register char delim)

Using "register" in argument lists is meaningless. Whether or not a
register is used is determined by the calling convention.

Linux/x86/gcc uses the stack by default; you can force the use of a
register using the "regparm" attribute, but that's seldom a good idea.

For local variables, it's almost always preferable to let the compiler
figure out when to use a register (and most modern compilers will just
ignore "register" specifiers).

This is the patch which I applied, hope that's right:
Index: index.c

RCS /grassrepository/grass6/lib/gis/index.c,v
retrieving revision 2.0
diff -u -r2.0 index.c
--- index.c 9 Nov 2004 12:18:38 -0000 2.0
+++ index.c 3 Jan 2006 06:32:53 -0000
@@ -1,3 +1,5 @@
+/* TODO: should this go into strings.c ? */
+
#include "gis.h"

@@ -12,8 +14,8 @@
  */

char *
-G_index (str, delim)
- register char *str, delim;
+G_index (char *str, int delim)
+
{
     while (*str && *str != delim)
        str++;
@@ -34,10 +36,10 @@
  */

char *
-G_rindex (str, delim)
- register char *str, delim;
+G_rindex (char *str, int delim)
+
{
- register char *p;
+ char *p;

     p = NULL;
     while (*str)

> ####################
> another one in lib/gis
>
> /* unctrl.c */
> char *G_unctrl(int);
>
> definition in unctrl.c
> char *G_unctrl(unsigned char c);

Change the implementation to match the declaration in gisdefs.h. The
K&R version will expect an int to have been passed, as that's the
smallest integral type that K&R allows.

Here's the patch which I applied:

Index: unctrl.c

RCS /grassrepository/grass6/lib/gis/unctrl.c,v
retrieving revision 2.0
diff -u -r2.0 unctrl.c
--- unctrl.c 9 Nov 2004 12:22:58 -0000 2.0
+++ unctrl.c 3 Jan 2006 06:32:53 -0000
@@ -9,13 +9,13 @@
  * represented by ctrl-C, e.g., control A is represented by ctrl-A. 0177 is
  * represented by DEL/RUB. Normal characters remain unchanged.
  *
- * \param c
+ * \param int c
  * \return char *
  */

char *
-G_unctrl(c)
- unsigned char c;
+G_unctrl (int c)
+
{
     static char buf[20];

> ####################
> imagery/open.c
>
> static error (char *group, char *file, char *msga, char *msgb)
>
> The compiler complains about absent return type:
> open.c:12: warning: return type defaults to `int'
> open.c: In function `error':
> open.c:17: warning: control reaches end of non-void function

Change it to "static void error(...)".

done.

> ####################
> Unrelated, I found that by chance:
>
> bartok:gis[6941.292] grep 'floor()\|ceil()' *
> align_window.c: double floor(), ceil();
> color_range.c: double floor(), ceil();
>
> I guess that it is no good idea to have it
> defined, right?

Use "#include <math.h>" to get the prototypes.

Done as well.

Thanks, Glynn, for the explanations.

Markus