Bug#703540: Inconsistent use of _GNU_SOURCE

Message ID 3212285.GF4Btbehao@sven-desktop (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Sven Eckelmann March 21, 2013, 8:24 a.m. UTC
  tags 703540 + pending patch
thanks

On Wednesday 20 March 2013 17:51:54 you wrote:
> While building the package using our research compiler infrastructure we
> noticed conflicting types being used in the linked executable. This is due
> to _GNU_SOURCE being defined in linux/route.c and posix/unix_socket.c, but
> not in any other file. As a result, system headers expand to conflicting
> declarations. (This was at least noticed for the sendto function, but may
> extend to others.)
> 
> Either all or no file should #define _GNU_SOURCE.

Please add information how to reproduce this the next time you are adding such 
such a bug. Now I can just assume what you are writing is true (even when the 
man page about sendto says otherwise). Not knowing how to reproduce it in the 
best possible way just makes it harder for everyone to check the impact of the 
problem.

I've forwarded it to the upstream maintainer and attached the change for 
Debian.

Kind regards,
	Sven
  

Comments

Michael Tautschnig March 29, 2013, 11:48 a.m. UTC | #1
Hi Sven, hi all,

[...]
> > Either all or no file should #define _GNU_SOURCE.
> 
> Please add information how to reproduce this the next time you are adding such 
> such a bug. Now I can just assume what you are writing is true (even when the 
> man page about sendto says otherwise). Not knowing how to reproduce it in the 
> best possible way just makes it harder for everyone to check the impact of the 
> problem.
> 

Just one question before I elaborate a bit: what information in the man page are
you referring to? I can't quite seem to see anything mentioning _GNU_SOURCE?

I fully understand your concerns and indeed it is a problem that I cannot quite
provide a concrete counterexample witnessing the problem. It may even be the
cast that, at present, this is only a potential problem and not a real one. It's
much like a compiler warning: ok to be ignored if you are doing it intentionally
and you are 100% sure you know what you are doing. In all other cases, however,
it is likely worth fixing, as the problem can only ever be found by link-time
type checking, which usual compilers can't do. Even if done, there is some
non-trivial effort required to tracing back the type inconsistency to
inconsistent order of #include or a missing #define.

The most I can provide right now is all the scripts that suffice to reproduce
the build results and error logs, to be found at
https://github.com/tautschnig/cprover-debian

> I've forwarded it to the upstream maintainer and attached the change for 
> Debian.
> 
[...]

Thanks!

Best,
Michael
  
Antonio Quartulli March 29, 2013, 12:37 p.m. UTC | #2
Hi Michael,

On Fri, Mar 29, 2013 at 11:48:16AM +0000, Michael Tautschnig wrote:
> Hi Sven, hi all,
> 
> [...]
> > > Either all or no file should #define _GNU_SOURCE.
> > 
> > Please add information how to reproduce this the next time you are adding such 
> > such a bug. Now I can just assume what you are writing is true (even when the 
> > man page about sendto says otherwise). Not knowing how to reproduce it in the 
> > best possible way just makes it harder for everyone to check the impact of the 
> > problem.
> > 
> 
> Just one question before I elaborate a bit: what information in the man page are
> you referring to? I can't quite seem to see anything mentioning _GNU_SOURCE?
> 
> I fully understand your concerns and indeed it is a problem that I cannot quite
> provide a concrete counterexample witnessing the problem. It may even be the
> cast that, at present, this is only a potential problem and not a real one. It's
> much like a compiler warning: ok to be ignored if you are doing it intentionally
> and you are 100% sure you know what you are doing. In all other cases, however,
> it is likely worth fixing, as the problem can only ever be found by link-time
> type checking, which usual compilers can't do. Even if done, there is some
> non-trivial effort required to tracing back the type inconsistency to
> inconsistent order of #include or a missing #define.
> 
> The most I can provide right now is all the scripts that suffice to reproduce
> the build results and error logs, to be found at
> https://github.com/tautschnig/cprover-debian
> 
> > I've forwarded it to the upstream maintainer and attached the change for 
> > Debian.
> > 
> [...]
> 
> Thanks!
> 


Elektra provided a patch to fix this issue and it has been applied already.
The patch and the related "Applied!" message have been posted to the batman ml,
maybe you are not subscribed and you did not get them?

Cheers,
  
Sven Eckelmann April 2, 2013, 9:10 a.m. UTC | #3
On Friday 29 March 2013 11:48:16 Michael Tautschnig wrote:
> Hi Sven, hi all,
> 
> [...]
> 
> > > Either all or no file should #define _GNU_SOURCE.
> > 
> > Please add information how to reproduce this the next time you are adding
> > such such a bug. Now I can just assume what you are writing is true (even
> > when the man page about sendto says otherwise).
[...]
> Just one question before I elaborate a bit: what information in the man page
> are you referring to? I can't quite seem to see anything mentioning
> _GNU_SOURCE?

Yes, this was exactly the thing I was pointing out:

> > > (This was at least noticed for the sendto function, but may extend to
> > > others.)

Kind regards,
	Sven
  

Patch

From 5041828ddb165b1c72c7fc4345aa6e0bec7abfa5 Mon Sep 17 00:00:00 2001
From: Sven Eckelmann <sven@narfation.org>
Date: Thu, 21 Mar 2013 09:21:48 +0100
Subject: [PATCH] Define _GNU_SOURCE for all POSIX target source files

Defining _GNU_SOURCE in source files differently can result in conflicting
types.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 debian/changelog                       |    3 +++
 debian/patches/define_gnu_source.patch |   43 ++++++++++++++++++++++++++++++++
 debian/patches/series                  |    1 +
 3 files changed, 47 insertions(+)
 create mode 100644 debian/patches/define_gnu_source.patch
 create mode 100644 debian/patches/series

diff --git a/debian/changelog b/debian/changelog
index 31e69a6..5d08680 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,6 +2,9 @@  batmand (0.3.2+50+g06537ed-2) UNRELEASED; urgency=low
 
   * Upgraded to policy 3.9.4, no changes required
   * Remove obsolete DM-Upload-Allowed in debian/control
+  * debian/patches:
+    - Add define_gnu_source.patch, Define _GNU_SOURCE for all POSIX target
+      source files (Closes: #703540)
 
  -- Sven Eckelmann <sven@narfation.org>  Wed, 19 Sep 2012 08:53:36 +0200
 
diff --git a/debian/patches/define_gnu_source.patch b/debian/patches/define_gnu_source.patch
new file mode 100644
index 0000000..d81235f
--- /dev/null
+++ b/debian/patches/define_gnu_source.patch
@@ -0,0 +1,43 @@ 
+Description: Define _GNU_SOURCE for all POSIX target source files
+ Defining _GNU_SOURCE in source files differently can result in conflicting
+ types.
+Bug-Debian: #703540
+Author: Sven Eckelmann <sven@narfation.org>
+
+---
+diff --git a/Makefile b/Makefile
+index fe1f094f310469331b3a7ff32e0eb2859bc5c998..ce1e198863aa311d5572e205f68534320d8f8431 100755
+--- a/Makefile
++++ b/Makefile
+@@ -27,6 +27,7 @@ LINUX_OBJ =	linux/route.o linux/tun.o linux/kernel.o
+ 
+ ifeq ($(UNAME),Linux)
+ OS_OBJ =	$(LINUX_OBJ) $(POSIX_OBJ)
++CPPFLAGS +=	-D_GNU_SOURCE
+ endif
+ 
+ OBJ = batman.o originator.o schedule.o list-batman.o allocate.o bitarray.o hash.o profile.o ring_buffer.o hna.o $(OS_OBJ)
+diff --git a/linux/route.c b/linux/route.c
+index 119ebb2c08d0babfacdfa34ca38c74cc3160ff43..bceac95f1fc57c37bd18e1ada15d6f55582043a0 100644
+--- a/linux/route.c
++++ b/linux/route.c
+@@ -21,7 +21,6 @@
+ 
+ 
+ 
+-#define _GNU_SOURCE
+ #include <sys/ioctl.h>
+ #include <arpa/inet.h>    /* inet_ntop() */
+ #include <errno.h>
+diff --git a/posix/unix_socket.c b/posix/unix_socket.c
+index 5bed217e286c247278046967783a197781c9a59e..02a0a3a97284fbf803565e12cebd8b22d305e66f 100644
+--- a/posix/unix_socket.c
++++ b/posix/unix_socket.c
+@@ -21,7 +21,6 @@
+ 
+ 
+ 
+-#define _GNU_SOURCE
+ #include <sys/time.h>
+ #include <stdio.h>
+ #include <fcntl.h>
diff --git a/debian/patches/series b/debian/patches/series
new file mode 100644
index 0000000..f9b7991
--- /dev/null
+++ b/debian/patches/series
@@ -0,0 +1 @@ 
+define_gnu_source.patch
-- 
1.7.10.4