[PATCH] Typo in mapwms.c (and lots of warnings)

[Frank Warmerdam]

On Mon, 24 Jan 2005 09:46:51 +0100, Petter Reinholdtsen <pere at hungry.com> wrote:
> I recently started looking at the mapserver source, and discovered
> that it is compiled using very few warning flags enabled.  When
> enabling more warnings, I found several bugs.  This is about one of
> them.  I use this patch to enable more warnings:
>
> Index: aclocal.m4
> ===================================================================
> RCS file: /data2/cvsroot/mapserver/aclocal.m4,v
> retrieving revision 1.10
> diff -u -r1.10 aclocal.m4
> --- aclocal.m4  22 Jul 2003 20:05:25 -0000      1.10
> +++ aclocal.m4  24 Jan 2005 08:37:21 -0000
> @@ -33,20 +33,21 @@
>
>  AC_DEFUN(AC_COMPILER_WFLAGS,
>  [
> +       WFLAGS="-W -Wall -Wcast-align -Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes -Wpointer-arith -Wreturn-type"
>         # Remove -g from compile flags, we will add via CFG variable if
>         # we need it.
>         CXXFLAGS=`echo "$CXXFLAGS " | sed "s/-g //"`
>         CFLAGS=`echo "$CFLAGS " | sed "s/-g //"`
>
> -       # check for GNU compiler, and use -Wall
> +       # check for GNU compiler, and use warning flags
>         if test "$GCC" = "yes"; then
> -               C_WFLAGS="-Wall"
> -               CFLAGS="$CFLAGS -Wall"
> +               C_WFLAGS="$WFLAGS"
> +               CFLAGS="$CFLAGS $WFLAGS"
>                 AC_DEFINE(USE_GNUCC)
>         fi
>         if test "$GXX" = "yes"; then
> -               CXX_WFLAGS="-Wall"
> -               CXXFLAGS="$CXXFLAGS -Wall"
> +               CXX_WFLAGS="$WFLAGS"
> +               CXXFLAGS="$CXXFLAGS $WFLAGS"
>                 AC_DEFINE(USE_GNUCC)
>         fi
>         AC_SUBST(CXX_WFLAGS,$CXX_WFLAGS)
> @@ -55,16 +56,17 @@
>
>  AC_DEFUN(AC_COMPILER_WFLAGS_DEBUG,
>  [
> +       WFLAGS="-W -Wall -Wcast-align -Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes -Wpointer-arith -Wreturn-type"
>         # -g should already be in compile flags.
> -       # check for GNU compiler, and add -Wall
> +       # check for GNU compiler, and add warning flags
>         if test "$GCC" = "yes"; then
> -               C_WFLAGS="-Wall"
> -               CFLAGS="$CFLAGS -Wall"
> +               C_WFLAGS="$WFLAGS"
> +               CFLAGS="$CFLAGS $WFLAGS"
>                 AC_DEFINE(USE_GNUCC)
>         fi
>         if test "$GXX" = "yes"; then
> -               CXX_WFLAGS="-Wall"
> -               CXXFLAGS="$CXXFLAGS -Wall"
> +               CXX_WFLAGS="$WFLAGS"
> +               CXXFLAGS="$CXXFLAGS $WFLAGS"
>                 AC_DEFINE(USE_GNUCC)
>         fi
>         AC_SUBST(CXX_WFLAGS,$CXX_WFLAGS)
>
> This in turn report lots and lots of potential problems in the code.
> I would like to help getting rid of all these warnings, but will need
> commends and suggestions on how to best solve them.
>
> One of the warnings are this one.
>
>   mapwms.c:1959: warning: ordered comparison of pointer with integer zero
>
> The problem seem to be a missing index in the source, where the
> pointer to an array is compared with zero instead of the value in the
> array.  This patch solve the problem.  Please include it in a future
> version of mapserver.
>
> Index: mapwms.c
> ===================================================================
> RCS file: /data2/cvsroot/mapserver/mapwms.c,v
> retrieving revision 1.158
> diff -u -r1.158 mapwms.c
> --- mapwms.c    14 Jan 2005 05:03:01 -0000      1.158
> +++ mapwms.c    24 Jan 2005 08:38:46 -0000
> @@ -1955,7 +1958,7 @@
>       //free the stuff used for nested layers
>       for (i = 0; i < map->numlayers; i++)
>       {
> -       if (numNestedGroups > 0)
> +       if (numNestedGroups[i] > 0)
>         {
>           msFreeCharArray(nestedGroups[i], numNestedGroups[i]);
>         }

Peter,

I would suggest submitting these as bugzilla reports with the patches attached.
Likewise for the sortshp.c change, and the UNUSED changes.  I tried
applying the sortshp.c patch with the patch program and it didn't work.

Best regards,
--
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent