{"thread":{"id":"33642","subject":"[PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux","startedAt":"2013-04-27T18:46:17Z","lastAt":"2013-04-29T23:09:15Z","messageCount":5,"participants":["Ramsay Jones","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"215726","messageId":"517C1CF9.2080903@ramsay1.demon.co.uk","threadId":"33642","inReplyTo":null,"subject":"[PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2013-04-27T18:46:17Z","receivedAt":"2013-04-27T18:46:17Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nOn linux, when the build variable USE_NED_ALLOCATOR is set, gcc\nissues the following warnings:\n\n In file included from compat/nedmalloc/nedmalloc.c:63:\n .../malloc.c.h: In function 'mmap_resize':\n .../malloc.c.h:3762: warning: implicit declaration of function 'mremap'\n .../malloc.c.h: In function 'sys_trim':\n .../malloc.c.h:4195: warning: comparison between pointer and integer\n\nThe warnings are caused by the header not enabling the\n(conditional) declaration of the mremap() function. The declaration\ncan be enabled by defining the _GNU_SOURCE symbol prior to including\ncertain system header files. In particular, it may not be sufficient\nto simply define _GNU_SOURCE just prior to including the \nheader. (e.g. defining the symbol after including will\nbe completely ineffective.)\n\nIn order to suppress the warnings, we define the _GNU_SOURCE symbol\nat the start of the malloc.c.h header file.\n\nSigned-off-by: Ramsay Jones \n---\n compat/nedmalloc/malloc.c.h | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h\nindex fdcca82..5a44dea 100644\n--- a/compat/nedmalloc/malloc.c.h\n+++ b/compat/nedmalloc/malloc.c.h\n@@ -484,6 +484,10 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP\n #define DLMALLOC_VERSION 20804\n #endif /* DLMALLOC_VERSION */\n \n+#if defined(linux)\n+#define _GNU_SOURCE 1\n+#endif\n+\n #ifndef WIN32\n #ifdef _WIN32\n #define WIN32 1\n-- \n1.8.2\n"},{"id":"215802","messageId":"7v7gjmqwxn.fsf@alter.siamese.dyndns.org","threadId":"33642","inReplyTo":"517C1CF9.2080903@ramsay1.demon.co.uk","subject":"Re: [PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-28T19:25:56Z","receivedAt":"2013-04-28T19:25:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> On linux, when the build variable USE_NED_ALLOCATOR is set, gcc\n> issues the following warnings:\n>\n> In file included from compat/nedmalloc/nedmalloc.c:63:\n> .../malloc.c.h: In function 'mmap_resize':\n> .../malloc.c.h:3762: warning: implicit declaration of function 'mremap'\n> .../malloc.c.h: In function 'sys_trim':\n> .../malloc.c.h:4195: warning: comparison between pointer and integer\n>\n> The warnings are caused by the header not enabling the\n> (conditional) declaration of the mremap() function. The declaration\n> can be enabled by defining the _GNU_SOURCE symbol prior to including\n> certain system header files. In particular, it may not be sufficient\n> to simply define _GNU_SOURCE just prior to including the \n> header. (e.g. defining the symbol after including will\n> be completely ineffective.)\n>\n> In order to suppress the warnings, we define the _GNU_SOURCE symbol\n> at the start of the malloc.c.h header file.\n>\n> Signed-off-by: Ramsay Jones \n> ---\n\nHmmmm, is this even worth worrying about? I somehow thought this\nwas only meant to replace malloc on non GNU systems...\n\n> compat/nedmalloc/malloc.c.h | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h\n> index fdcca82..5a44dea 100644\n> --- a/compat/nedmalloc/malloc.c.h\n> +++ b/compat/nedmalloc/malloc.c.h\n> @@ -484,6 +484,10 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP\n> #define DLMALLOC_VERSION 20804\n> #endif /* DLMALLOC_VERSION */\n> \n> +#if defined(linux)\n> +#define _GNU_SOURCE 1\n> +#endif\n> +\n> #ifndef WIN32\n> #ifdef _WIN32\n> #define WIN32 1\n"},{"id":"215822","messageId":"20130429050616.GB8031@elie.Belkin","threadId":"33642","inReplyTo":"517C1CF9.2080903@ramsay1.demon.co.uk","subject":"Re: [PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-29T05:06:16Z","receivedAt":"2013-04-29T05:06:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRamsay Jones wrote:\n\n> --- a/compat/nedmalloc/malloc.c.h\n> +++ b/compat/nedmalloc/malloc.c.h\n> @@ -484,6 +484,10 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP\n> #define DLMALLOC_VERSION 20804\n> #endif /* DLMALLOC_VERSION */\n> \n> +#if defined(linux)\n> +#define _GNU_SOURCE 1\n> +#endif\n\nIf we want to do this, I'd suggest defining _GNU_SOURCE\nunconditionally. Other platforms using glibc would benefit from it,\ntoo, and other platforms not using glibc shouldn't care.\n\nAlternatively it could make sense to see if the latest version from\nNiall Douglas[1] works, which includes the following:\n\n\t#ifndef HAVE_MREMAP\n\t#ifdef linux\n\t#define HAVE_MREMAP 1\n\t#ifndef _GNU_SOURCE\n\t#define _GNU_SOURCE /* Turns on mremap() definition */\n\t#endif\n\t#else /* linux */\n\t#define HAVE_MREMAP 0\n\t#endif /* linux */\n\t#endif /* HAVE_MREMAP */\n\nIt looks equally wrong there, but since it comes from upstream we\ncould just punt to him if anyone runs into problems. ;-)\n\nMy two cents,\nJonathan\n\n[1] http://www.nedprod.com/programs/portable/nedmalloc/\n"},{"id":"215938","messageId":"517EFBB5.9040502@ramsay1.demon.co.uk","threadId":"33642","inReplyTo":"7v7gjmqwxn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2013-04-29T23:01:09Z","receivedAt":"2013-04-29T23:01:09Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n>> On linux, when the build variable USE_NED_ALLOCATOR is set, gcc\n>> issues the following warnings:\n>>\n>> In file included from compat/nedmalloc/nedmalloc.c:63:\n>> .../malloc.c.h: In function 'mmap_resize':\n>> .../malloc.c.h:3762: warning: implicit declaration of function 'mremap'\n>> .../malloc.c.h: In function 'sys_trim':\n>> .../malloc.c.h:4195: warning: comparison between pointer and integer\n>>\n>> The warnings are caused by the header not enabling the\n>> (conditional) declaration of the mremap() function. The declaration\n>> can be enabled by defining the _GNU_SOURCE symbol prior to including\n>> certain system header files. In particular, it may not be sufficient\n>> to simply define _GNU_SOURCE just prior to including the \n>> header. (e.g. defining the symbol after including will\n>> be completely ineffective.)\n>>\n>> In order to suppress the warnings, we define the _GNU_SOURCE symbol\n>> at the start of the malloc.c.h header file.\n>>\n>> Signed-off-by: Ramsay Jones \n>> ---\n> \n> Hmmmm, is this even worth worrying about? I somehow thought this\n> was only meant to replace malloc on non GNU systems...\n\nMaybe not. I certainly won't be using it on Linux. But I wanted to\ntest the previous patch on Linux and it didn't compile cleanly, so ...\n(It passed all tests BTW).\n\nATB,\nRamsay Jones\n"},{"id":"215940","messageId":"517EFD9B.4020608@ramsay1.demon.co.uk","threadId":"33642","inReplyTo":"20130429050616.GB8031@elie.Belkin","subject":"Re: [PATCH 6/6] compat/nedmalloc: Fix compiler warnings on linux","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2013-04-29T23:09:15Z","receivedAt":"2013-04-29T23:09:15Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jonathan Nieder wrote:\n> Hi,\n> \n> Ramsay Jones wrote:\n> \n>> --- a/compat/nedmalloc/malloc.c.h\n>> +++ b/compat/nedmalloc/malloc.c.h\n>> @@ -484,6 +484,10 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP\n>> #define DLMALLOC_VERSION 20804\n>> #endif /* DLMALLOC_VERSION */\n>>\n>> +#if defined(linux)\n>> +#define _GNU_SOURCE 1\n>> +#endif\n> \n> If we want to do this, I'd suggest defining _GNU_SOURCE\n> unconditionally. Other platforms using glibc would benefit from it,\n> too, and other platforms not using glibc shouldn't care.\n\nMy original version was unconditional, then I came over all\nconservative ... :-D\n\nActually, in order to reduce the chance of future breakage, it\nshould really be placed at the beginning of the translation unit\n(i.e. at the start of nedmalloc.c, before *any* #includes).\n\nATB,\nRamsay Jones\n"}]}