threads / patch / 33642

patch, 6 partscompat/nedmalloc: Fix compiler warnings on linux

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

## tl;dr

5 messages between Apr 27, 2013 and Apr 29, 2013. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Ramsay Jones· Apr 27, 2013, 18:46 UTC · lore

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(+)
Show changes to compat/nedmalloc/malloc.c.h +4 −0
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
Junio C Hamano· Apr 28, 2013, 19:25 UTC · re: Ramsay Jones · lore

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

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
Show 22 quoted lines
> 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...

Show 18 quoted lines
>  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
Ramsay Jones· Apr 29, 2013, 23:01 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 27 quoted lines
> 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

Jonathan Nieder· Apr 29, 2013, 05:06 UTC · re: Ramsay Jones · lore

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

Hi,
Ramsay Jones wrote:
Show 9 quoted lines
> --- 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/
Ramsay Jones· Apr 29, 2013, 23:09 UTC · re: Jonathan Nieder · lore

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

Jonathan Nieder wrote:
Show 17 quoted lines
> 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

← back to recent threads