Message ID | 1407176414-13376-1-git-send-email-benoit.thebaudeau@advansee.com |
---|---|
State | Superseded |
Headers | show |
Dear Benoît Thébaudeau, On Mon, 4 Aug 2014 20:20:14 +0200, Benoît Thébaudeau wrote: > diff --git a/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch b/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch > new file mode 100644 > index 0000000..10b826c > --- /dev/null > +++ b/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch All patches should have a description + Signed-off-by. Also, you should use the proper naming convention for patches: <package>-<sequencenumber>-<description>.patch I know there's already a patch for dhcpdump that doesn't follow this convention, but you're invited to rename it as well :-) > @@ -0,0 +1,12 @@ > +diff -Nrdup dhcpdump-1.8.orig/dhcpdump.c dhcpdump-1.8/dhcpdump.c > +--- dhcpdump-1.8.orig/dhcpdump.c 2008-06-24 05:26:52.000000000 +0200 > ++++ dhcpdump-1.8/dhcpdump.c 2011-05-31 19:22:15.987388498 +0200 > +@@ -26,7 +26,7 @@ > + #include <regex.h> > + #include "dhcp_options.h" > + > +-#ifndef HAVE_STRSEP > ++#ifndef _BSD_SOURCE Is _BSD_SOURCE really meant to be tested within source code? I thought it was more the application or library that would #define _BSD_SOURCE or #define _GNU_SOURCE to tell the C library which functions should be made visible. But well, if strsep() depends on _BSD_SOURCE being defined, I agree that testing _BSD_SOURCE is a way of knowing whether it's available or not. Are there some other opinions about this? Thomas
On Monday, August 4, 2014 9:36:31 PM, Thomas Petazzoni wrote: > Dear Benoît Thébaudeau, > > On Mon, 4 Aug 2014 20:20:14 +0200, Benoît Thébaudeau wrote: > > > diff --git a/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch > > b/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch > > new file mode 100644 > > index 0000000..10b826c > > --- /dev/null > > +++ b/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch > > All patches should have a description + Signed-off-by. Also, you should > use the proper naming convention for patches: > > <package>-<sequencenumber>-<description>.patch > > I know there's already a patch for dhcpdump that doesn't follow this > convention, but you're invited to rename it as well :-) Will do. > > @@ -0,0 +1,12 @@ > > +diff -Nrdup dhcpdump-1.8.orig/dhcpdump.c dhcpdump-1.8/dhcpdump.c > > +--- dhcpdump-1.8.orig/dhcpdump.c 2008-06-24 05:26:52.000000000 +0200 > > ++++ dhcpdump-1.8/dhcpdump.c 2011-05-31 19:22:15.987388498 +0200 > > +@@ -26,7 +26,7 @@ > > + #include <regex.h> > > + #include "dhcp_options.h" > > + > > +-#ifndef HAVE_STRSEP > > ++#ifndef _BSD_SOURCE > > Is _BSD_SOURCE really meant to be tested within source code? I thought > it was more the application or library that would #define _BSD_SOURCE > or #define _GNU_SOURCE to tell the C library which functions should be > made visible. But well, if strsep() depends on _BSD_SOURCE being > defined, I agree that testing _BSD_SOURCE is a way of knowing whether > it's available or not. > > Are there some other opinions about this? Yes, _BSD_SOURCE is supposed to be #define-d before all #include-s in order to select the set of features. But here, we do not want to enforce any set of features, but only to detect if strsep() is supported by the default feature set, so we have no other choice than testing _BSD_SOURCE, just like the libc does. Another way would be to define or not HAVE_STRSEP from dhcpdump.mk based on some criterion (which one? just forced?), but this seems less reliable. Best regards, Benoît
diff --git a/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch b/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch new file mode 100644 index 0000000..10b826c --- /dev/null +++ b/package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch @@ -0,0 +1,12 @@ +diff -Nrdup dhcpdump-1.8.orig/dhcpdump.c dhcpdump-1.8/dhcpdump.c +--- dhcpdump-1.8.orig/dhcpdump.c 2008-06-24 05:26:52.000000000 +0200 ++++ dhcpdump-1.8/dhcpdump.c 2011-05-31 19:22:15.987388498 +0200 +@@ -26,7 +26,7 @@ + #include <regex.h> + #include "dhcp_options.h" + +-#ifndef HAVE_STRSEP ++#ifndef _BSD_SOURCE + #include "strsep.c" + #endif +
Use the official _BSD_SOURCE feature test macro instead of the meaningless HAVE_STRSEP macro in order to detect the availability of strsep(). This allows toolchains supporting strsep() to use it instead of the custom implementation from dhcpdump, which also avoids the following error with some toolchains: In file included from dhcpdump.c:30:0: dhcpdump.c: At top level: strsep.c:65:23: error: register name not specified for ‘delim’ register const char *delim; ^ Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> --- package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 package/dhcpdump/dhcpdump-1.8-fix-strsep-feature-test.patch