[PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux

[PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux

[Ramsay Jones]

On linux, when the build variable USE_NED_ALLOCATOR is set, gcc
issues the following warnings:

    In file included from compat/nedmalloc/nedmalloc.c:63:
    .../malloc.c.h: In function 'mmap_resize':
    .../malloc.c.h:3762: warning: implicit declaration of function 'mremap'
    .../malloc.c.h: In function 'sys_trim':
    .../malloc.c.h:4195: warning: comparison between pointer and integer

The warnings are caused by the <sys/mman.h> header not enabling the
(conditional) declaration of the mremap() function.  The declaration
can be enabled by defining the _GNU_SOURCE symbol prior to including
certain system header files. In particular, it may not be sufficient
to simply define _GNU_SOURCE just prior to including the <sys/mman.h>
header. (e.g. defining the symbol after including <sys/types.h> will
be completely ineffective.)

In order to suppress the warnings, we define the _GNU_SOURCE symbol
at the start of the malloc.c.h header file.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 compat/nedmalloc/malloc.c.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
index fdcca82..5a44dea 100644
--- a/compat/nedmalloc/malloc.c.h
+++ b/compat/nedmalloc/malloc.c.h
@@ -484,6 +484,10 @@ MAX_RELEASE_CHECK_RATE   default: 4095 unless not HAVE_MMAP
 #define DLMALLOC_VERSION 20804
 #endif /* DLMALLOC_VERSION */
 
+#if defined(linux)
+#define _GNU_SOURCE 1
+#endif
+
 #ifndef WIN32
 #ifdef _WIN32
 #define WIN32 1
-- 
1.8.2

Re: [PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> On linux, when the build variable USE_NED_ALLOCATOR is set, gcc
> issues the following warnings:
>
>     In file included from compat/nedmalloc/nedmalloc.c:63:
>     .../malloc.c.h: In function 'mmap_resize':
>     .../malloc.c.h:3762: warning: implicit declaration of function 'mremap'
>     .../malloc.c.h: In function 'sys_trim':
>     .../malloc.c.h:4195: warning: comparison between pointer and integer
>
> The warnings are caused by the <sys/mman.h> header not enabling the
> (conditional) declaration of the mremap() function.  The declaration
> can be enabled by defining the _GNU_SOURCE symbol prior to including
> certain system header files. In particular, it may not be sufficient
> to simply define _GNU_SOURCE just prior to including the <sys/mman.h>
> header. (e.g. defining the symbol after including <sys/types.h> will
> be completely ineffective.)
>
> In order to suppress the warnings, we define the _GNU_SOURCE symbol
> at the start of the malloc.c.h header file.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---

Hmmmm, is this even worth worrying about?  I somehow thought this
was only meant to replace malloc on non GNU systems...

>  compat/nedmalloc/malloc.c.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
> index fdcca82..5a44dea 100644
> --- a/compat/nedmalloc/malloc.c.h
> +++ b/compat/nedmalloc/malloc.c.h
> @@ -484,6 +484,10 @@ MAX_RELEASE_CHECK_RATE   default: 4095 unless not HAVE_MMAP
>  #define DLMALLOC_VERSION 20804
>  #endif /* DLMALLOC_VERSION */
>  
> +#if defined(linux)
> +#define _GNU_SOURCE 1
> +#endif
> +
>  #ifndef WIN32
>  #ifdef _WIN32
>  #define WIN32 1

Re: [PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux

[Ramsay Jones]

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> On linux, when the build variable USE_NED_ALLOCATOR is set, gcc
>> issues the following warnings:
>>
>>     In file included from compat/nedmalloc/nedmalloc.c:63:
>>     .../malloc.c.h: In function 'mmap_resize':
>>     .../malloc.c.h:3762: warning: implicit declaration of function 'mremap'
>>     .../malloc.c.h: In function 'sys_trim':
>>     .../malloc.c.h:4195: warning: comparison between pointer and integer
>>
>> The warnings are caused by the <sys/mman.h> header not enabling the
>> (conditional) declaration of the mremap() function.  The declaration
>> can be enabled by defining the _GNU_SOURCE symbol prior to including
>> certain system header files. In particular, it may not be sufficient
>> to simply define _GNU_SOURCE just prior to including the <sys/mman.h>
>> header. (e.g. defining the symbol after including <sys/types.h> will
>> be completely ineffective.)
>>
>> In order to suppress the warnings, we define the _GNU_SOURCE symbol
>> at the start of the malloc.c.h header file.
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>> ---
> 
> Hmmmm, is this even worth worrying about?  I somehow thought this
> was only meant to replace malloc on non GNU systems...

Maybe not. I certainly won't be using it on Linux. But I wanted to
test the previous patch on Linux and it didn't compile cleanly, so ...
(It passed all tests BTW).

ATB,
Ramsay Jones

Re: [PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux

[Jonathan Nieder]

Hi,

Ramsay Jones wrote:

> --- a/compat/nedmalloc/malloc.c.h
> +++ b/compat/nedmalloc/malloc.c.h
> @@ -484,6 +484,10 @@ MAX_RELEASE_CHECK_RATE   default: 4095 unless not HAVE_MMAP
>  #define DLMALLOC_VERSION 20804
>  #endif /* DLMALLOC_VERSION */
> 
> +#if defined(linux)
> +#define _GNU_SOURCE 1
> +#endif

If we want to do this, I'd suggest defining _GNU_SOURCE
unconditionally.  Other platforms using glibc would benefit from it,
too, and other platforms not using glibc shouldn't care.

Alternatively it could make sense to see if the latest version from
Niall Douglas[1] works, which includes the following:

	#ifndef HAVE_MREMAP
	#ifdef linux
	#define HAVE_MREMAP 1
	#ifndef _GNU_SOURCE
	#define _GNU_SOURCE /* Turns on mremap() definition */
	#endif
	#else   /* linux */
	#define HAVE_MREMAP 0
	#endif  /* linux */
	#endif  /* HAVE_MREMAP */

It looks equally wrong there, but since it comes from upstream we
could just punt to him if anyone runs into problems. ;-)

My two cents,
Jonathan

[1] http://www.nedprod.com/programs/portable/nedmalloc/

Re: [PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux

[Ramsay Jones]

Jonathan Nieder wrote:
> Hi,
> 
> Ramsay Jones wrote:
> 
>> --- a/compat/nedmalloc/malloc.c.h
>> +++ b/compat/nedmalloc/malloc.c.h
>> @@ -484,6 +484,10 @@ MAX_RELEASE_CHECK_RATE   default: 4095 unless not HAVE_MMAP
>>  #define DLMALLOC_VERSION 20804
>>  #endif /* DLMALLOC_VERSION */
>>
>> +#if defined(linux)
>> +#define _GNU_SOURCE 1
>> +#endif
> 
> If we want to do this, I'd suggest defining _GNU_SOURCE
> unconditionally.  Other platforms using glibc would benefit from it,
> too, and other platforms not using glibc shouldn't care.

My original version was unconditional, then I came over all
conservative ... :-D

Actually, in order to reduce the chance of future breakage, it
should really be placed at the beginning of the translation unit
(i.e. at the start of nedmalloc.c, before *any* #includes).

ATB,
Ramsay Jones