re: winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Dan Kegel]

Doesn't build here:

ccache gcc -c -I. -I. -I../../include -I../../include  -D__WINESRC__
-D_WINMM_ -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing
-Wdeclaration-after-statement -Wempty-body -Wstrict-prototypes
-Wtype-limits -Wunused-but-set-parameter -Wwrite-strings
-fno-omit-frame-pointer -Wpointer-arith -Wlogical-op
-I/usr/include/freetype2  -g -O0 -Werror -Wno-unused-but-set-variable
-Wno-unused-but-set-parameter  -o lolvldrv.o lolvldrv.c
lolvldrv.c: In function 'MMDRV_InitPerType':
lolvldrv.c:406:13: error: pointer of type 'void *' used in arithmetic
[-Werror=pointer-arith]
lolvldrv.c:411:13: error: pointer of type 'void *' used in arithmetic
[-Werror=pointer-arith]
cc1: all warnings being treated as errors
make[1]: Leaving directory
`/home/bob/wineslave.dir/sandbox/slave/runtests-default/build/dlls/winmm'
make[1]: *** [lolvldrv.o] Error 1
make: *** [dlls/winmm] Error 2

http://buildbot.kegel.com/builders/runtests-default/builds/874

winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Frédéric Delanoy]

2011/10/22 Dan Kegel :
> Doesn't build here:
>
> ccache gcc -c -I. -I. -I../../include -I../../include  -D__WINESRC__
> -D_WINMM_ -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing
> -Wdeclaration-after-statement -Wempty-body -Wstrict-prototypes
> -Wtype-limits -Wunused-but-set-parameter -Wwrite-strings
> -fno-omit-frame-pointer -Wpointer-arith -Wlogical-op
> -I/usr/include/freetype2  -g -O0 -Werror -Wno-unused-but-set-variable
> -Wno-unused-but-set-parameter  -o lolvldrv.o lolvldrv.c
> lolvldrv.c: In function 'MMDRV_InitPerType':
> lolvldrv.c:406:13: error: pointer of type 'void *' used in arithmetic
> [-Werror=pointer-arith]
> lolvldrv.c:411:13: error: pointer of type 'void *' used in arithmetic
> [-Werror=pointer-arith]
> cc1: all warnings being treated as errors
> make[1]: Leaving directory
> `/home/bob/wineslave.dir/sandbox/slave/runtests-default/build/dlls/winmm'
> make[1]: *** [lolvldrv.o] Error 1
> make: *** [dlls/winmm] Error 2

Right.
OK thx. Should probably restructure that section as well (and add
-Werror to my CFLAGS)

winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Dan Kegel]

2011/10/22 Frédéric Delanoy :
> OK thx. Should probably restructure that section as well (and add
> -Werror to my CFLAGS)

How would people feel if configure turned on -Werror
when it's safe to if building from git?  Something like this:

--- a/configure.ac
+++ b/configure.ac
@@ -1715,6 +1715,16 @@ then
   if test "x$enable_maintainer_mode" = "xyes"
   then
       WINE_TRY_CFLAGS([-Werror])
+  else
+    dnl Enable -Werror when building from git on 32 bit x86 with gcc
+    case $host_cpu in
+      *i[[3456789]]86*)
+        if test "x${GCC}" = "xyes" && test -d $srcdir/.git
+        then
+          WINE_TRY_CFLAGS([-Werror])
+        fi
+      ;;
+    esac
   fi

dnl Check for ms_hook_prologue support

winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Jerome Leclanche]

2011/10/23 Dan Kegel :
> 2011/10/22 Frédéric Delanoy :
> > OK thx. Should probably restructure that section as well (and add
> > -Werror to my CFLAGS)
>
> How would people feel if configure turned on -Werror
> when it's safe to if building from git?  Something like this:
>
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1715,6 +1715,16 @@ then
>   if test "x$enable_maintainer_mode" = "xyes"
>   then
>       WINE_TRY_CFLAGS([-Werror])
> - else
> - dnl Enable -Werror when building from git on 32 bit x86 with gcc
> - case $host_cpu in
> - *i[[3456789]]86*)
> - if test "x${GCC}" = "xyes" && test -d $srcdir/.git
> - then
> - WINE_TRY_CFLAGS([-Werror])
> - fi
> - ;;
> - esac
>
> fi
>
> dnl Check for ms_hook_prologue support

It's still unsafe, you're limited to a subset of gcc versions. Not worth it.

JL

winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Dan Kegel]

2011/10/23 Jerome Leclanche :
> > How would people feel if configure turned on -Werror
> > when it's safe to if building from git?  Something like this...
>
> It's still unsafe, you're limited to a subset of gcc versions.

That's not too hard to check for:

--- a/configure.ac
+++ b/configure.ac
@@ -1715,6 +1715,19 @@ then
   if test "x$enable_maintainer_mode" = "xyes"
   then
       WINE_TRY_CFLAGS([-Werror])
+  else
+    dnl Enable -Werror when building from git on 32 bit x86 with a
known good version of gcc
+    case $host_cpu in
+      *i[[3456789]]86*)
+        if test "x${GCC}" = "xyes" && test -d $srcdir/.git
+        then
+          case `$(CC) -v 2>&1 | grep "gcc version" | sed 's/gcc
version //' ` in
+          4.[3456].*)
+            WINE_TRY_CFLAGS([-Werror]) ;;
+          esac
+        fi
+      ;;
+    esac
   fi

dnl Check for ms_hook_prologue support
> Not worth it.

In which sense?   Is it too hard to make it safe?

If it safely cuts down on the number of times people
have to resubmit patches, why not do it?
- Dan

winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Dan Kegel]

Debugged version:

--- a/configure.ac
+++ b/configure.ac
@@ -1715,6 +1715,20 @@ then
   if test "x$enable_maintainer_mode" = "xyes"
   then
       WINE_TRY_CFLAGS([-Werror])
+  else
+    dnl Enable -Werror when building from git on 32 bit x86 with a
known good version of gcc
+    case $host_cpu in
+      *i[[3456789]]86*)
+        if test "x${GCC}" = "xyes" && test -d $srcdir/.git
+        then
+          gcc_version=`$CC -v 2>&1 | grep "gcc version" | sed 's/gcc
version //'`
+          case $gcc_version in
+          4.[[3456]].*)
+            WINE_TRY_CFLAGS([-Werror]) ;;
+          esac
+        fi
+      ;;
+    esac
   fi

dnl Check for ms_hook_prologue support

winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Austin English]

On Sun, Oct 23, 2011 at 08:43, Dan Kegel  wrote:
> Debugged version:
>
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1715,6 +1715,20 @@ then
>   if test "x$enable_maintainer_mode" = "xyes"
>   then
>       WINE_TRY_CFLAGS([-Werror])
> - else
> - dnl Enable -Werror when building from git on 32 bit x86 with a
>
> known good version of gcc
> - case $host_cpu in
> - *i[[3456789]]86*)
> - if test "x${GCC}" = "xyes" && test -d $srcdir/.git
> - then
> - gcc_version=`$CC -v 2>&1 | grep "gcc version" | sed 's/gcc
>
> version //'`
> - case $gcc_version in
> - 4.[[3456]].*)
> - WINE_TRY_CFLAGS([-Werror]) ;;
> - esac
> - fi
> - ;;
> - esac
>
> fi
>
> dnl Check for ms_hook_prologue support

This has come up a few times before, and it's already been discussed,
-Werror will not go in by default.

winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Dan Kegel]

On Sun, Oct 23, 2011 at 3:18 PM, Austin English  wrote:
> This has come up a few times before, and it's already been discussed,
> -Werror will not go in by default.

Well, yes, several times, but I wasn't sure if that
was a "never under any circumstances",
or if the objections could be picked off one by one.
Evidently it's the former.

But that's ok, that's what buildbot is for, and there aren't
that many new wine developers stubbing their toes on
this at any one time.

winmm: Avoid casting return value of Heap(Re)Alloc calls.

[Michael Stefaniuc]

On 10/23/2011 02:41 PM, Dan Kegel wrote:
> 2011/10/22 Frédéric Delanoy :
> > OK thx. Should probably restructure that section as well (and add
> > -Werror to my CFLAGS)
>
> How would people feel if configure turned on -Werror
> when it's safe to if building from git?  Something like this:

This isn't safe at all. The quality of the gcc's and libraries used
vary. And newer gcc tend to add additional warnings, e.g. there are
still warnings with gcc-4.6.

What helps is to pass "-s" to make to suppress the normal make output.
That way it is waaay easier to see warnings/errors.
$ git am /tmp/winmm:\ Avoid\ casting\ return\ value\ of\
Heap(Re)Alloc\ calls..eml
$ make -j4 -s
dlls/winmm/lolvldrv.c: In function ‘MMDRV_InitPerType’:
dlls/winmm/lolvldrv.c:406:13: warning: pointer of type ‘void *’ used in
arithmetic [-Wpointer-arith]
dlls/winmm/lolvldrv.c:411:13: warning: pointer of type ‘void *’ used in
arithmetic [-Wpointer-arith]
Wine build complete.

The normal make output is too noisy to be useful.

bye
    michael
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1715,6 +1715,16 @@ then
>    if test "x$enable_maintainer_mode" = "xyes"
>    then
>        WINE_TRY_CFLAGS([-Werror])
> -
>
> else
> -
>
> dnl Enable -Werror when building from git on 32 bit x86 with gcc
> -
>
> case $host_cpu in
> -
>  *i[[3456789]]86*)
> -
>    if test "x${GCC}" = "xyes" && test -d $srcdir/.git
> -
>    then
> -
>      WINE_TRY_CFLAGS([-Werror])
> -
>    fi
> -
>  ;;
> -
>
> esac
> fi
>
> dnl Check for ms_hook_prologue support