Syslinux - Mar 2016 - [PATCH 5/5] installers: fix a MinGW redefinition warning

I get a redefinition warning on _GNU_SOURCE when compiling with MinGW, 
and while I could see that this #define was introduced in e4fc44 [1], 
but the reason to introduce it is not mentioned, and I can't really see 
a good reason to have it, especially as MSVC will happily compile that 
source. So far I have found no evidence that _GNU_SOURCE applies to 
memset/memmove/memcpy, which are the only calls I see in there that I 
could see impacted. Besides, for obvious compatibility reasons, I would 
advocate using -D_GNU_SOURCE as a compiler option if one really needs 
it, rather than have it in a source (which is what Syslinux already 
seems to do elsewhere).

[1] 
https://github.com/geneC/syslinux/commit/e4fc443f9b70f188963ff33e0a16ccb72a553540
-------------- next part --------------

On Wed, Feb 24, 2016 at 8:02 AM, Pete Batard via Syslinux
<syslinux at zytor.com> wrote:
> I get a redefinition warning on _GNU_SOURCE when compiling with MinGW, and
> while I could see that this #define was introduced in e4fc44 [1], but the
> reason to introduce it is not mentioned, and I can't really see a good
> reason to have it, especially as MSVC will happily compile that source. So
> far I have found no evidence that _GNU_SOURCE applies to
> memset/memmove/memcpy, which are the only calls I see in there that I could
> see impacted. Besides, for obvious compatibility reasons, I would advocate
> using -D_GNU_SOURCE as a compiler option if one really needs it, rather
than
> have it in a source (which is what Syslinux already seems to do elsewhere).
Could you provide details on this re-definition?  I see lots of use of
it in libinstaller/
> [1]
>
https://github.com/geneC/syslinux/commit/e4fc443f9b70f188963ff33e0a16ccb72a553540
>
> From 6dc3ad338cf740bf81e2148a35222485904ac615 Mon Sep 17 00:00:00 2001
> From: Pete Batard <pete at akeo.ie>
> Date: Tue, 23 Feb 2016 20:03:36 +0000
> Subject: [PATCH 5/5] installers: fix a MinGW redef warning
>
> ---
>  libinstaller/setadv.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/libinstaller/setadv.c b/libinstaller/setadv.c
> index 214f7fc..437583e 100644
> --- a/libinstaller/setadv.c
> +++ b/libinstaller/setadv.c
> @@ -14,13 +14,12 @@
>  /*
>   * setadv.c
>   *
> - * (Over)write a data item in the auxilliary data vector.  To
> + * (Over)write a data item in the auxiliary data vector.  To
>   * delete an item, set its length to zero.
>   *
>   * Return 0 on success, -1 on error, and set errno.
>   *
>   */
> -#define  _GNU_SOURCE
>
>  #include <stdio.h>
>  #include <stdlib.h>
> --
> 1.9.5.msysgit.1
-- 
-Gene

On 2016.03.06 15:39, Gene Cumm wrote:
> Could you provide details on this re-definition?  I see lots of use of
> it in libinstaller/
------------------------------------------------------------------------
Making all in syslinux/libinstaller
make[2]: Entering directory '/c/rufus/src/syslinux/libinstaller'
   CC       libinstaller_a-fs.o
   CC       libinstaller_a-setadv.o
setadv.c:23:0: warning: "_GNU_SOURCE" redefined
  #define  _GNU_SOURCE
  ^
<command-line>:0:0: note: this is the location of the previous definition
   CC       libinstaller_a-syslxmod.o
   AR       libinstaller.a
make[2]: Leaving directory '/c/rufus/src/syslinux/libinstaller'
------------------------------------------------------------------------

The reason is I use
  AC_DEFINE([_GNU_SOURCE], [], [Use GNU extensions])
in my configure.ac.

But as I also mentioned, it does look to me like the use of _GNU_SOURCE 
is inconsistent within the Syslinux project in the first place, as I'm 
pretty sure I've seen it defined as a -D compiler option in quite a few 
instances, which I think is the better way of requesting the use of GNU 
extensions.

Also, it doesn't look to me like there's anything in that file right now
that justifies the use of _GNU_SOURCE. Looking at the git history, it 
appears that _GNU_SOURCE was introduced to use extensions on some 
function calls that have since been moved into another file.

So really, it doesn't make much sense to me to still have _GNU_SOURCE in 
setadv.c...

Regards,

/Pete