| Message ID | 20171026172857.8683-1-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 10/26/17 12:28 PM, Luis R. Rodriguez wrote: > ./configure will leave traces of a complaint on config.log when > _BSD_SOURCE is defined but not _DEFAULT_SOURCE. _BSD_SOURCE is > deprecated so if defining _BSD_SOURCE also define _DEFAULT_SOURCE. > > From config.log: > /usr/include/features.h:183:3: warning: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Wcpp] Ok. Or perhaps a more clear rationale is the preadv(5) manpage itself: preadv(), pwritev(): since glibc 2.19: _DEFAULT_SOURCE Glibc 2.19 and earlier: _BSD_SOURCE But this is just a feature test macro in a configure script. What about the code itself, how do we handle this when we actually use preadv() in the codebase? -Eric > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > --- > m4/package_libcdev.m4 | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 > index fa5b63978797..af2a9631b8ba 100644 > --- a/m4/package_libcdev.m4 > +++ b/m4/package_libcdev.m4 > @@ -137,6 +137,7 @@ AC_DEFUN([AC_HAVE_PREADV], > [ AC_MSG_CHECKING([for preadv]) > AC_TRY_LINK([ > #define _BSD_SOURCE > +#define _DEFAULT_SOURCE > #include <sys/uio.h> > ], [ > preadv(0, 0, 0, 0); > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 2, 2017 at 1:04 PM, Eric Sandeen <sandeen@sandeen.net> wrote: > On 10/26/17 12:28 PM, Luis R. Rodriguez wrote: >> ./configure will leave traces of a complaint on config.log when >> _BSD_SOURCE is defined but not _DEFAULT_SOURCE. _BSD_SOURCE is >> deprecated so if defining _BSD_SOURCE also define _DEFAULT_SOURCE. >> >> From config.log: >> /usr/include/features.h:183:3: warning: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Wcpp] > > Ok. Or perhaps a more clear rationale is the preadv(5) manpage > itself: > > preadv(), pwritev(): > since glibc 2.19: > _DEFAULT_SOURCE > Glibc 2.19 and earlier: > _BSD_SOURCE > > But this is just a feature test macro in a configure script. > > What about the code itself, how do we handle this when we > actually use preadv() in the codebase? Its not clear to me what the issue would be by defining this extra _DEFAULT_SOURCE when _BSD_SOURCE is defined. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Nov 2, 2017, at 3:22 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: > >> On Thu, Nov 2, 2017 at 1:04 PM, Eric Sandeen <sandeen@sandeen.net> wrote: >>> On 10/26/17 12:28 PM, Luis R. Rodriguez wrote: >>> ./configure will leave traces of a complaint on config.log when >>> _BSD_SOURCE is defined but not _DEFAULT_SOURCE. _BSD_SOURCE is >>> deprecated so if defining _BSD_SOURCE also define _DEFAULT_SOURCE. >>> >>> From config.log: >>> /usr/include/features.h:183:3: warning: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Wcpp] >> >> Ok. Or perhaps a more clear rationale is the preadv(5) manpage >> itself: >> >> preadv(), pwritev(): >> since glibc 2.19: >> _DEFAULT_SOURCE >> Glibc 2.19 and earlier: >> _BSD_SOURCE >> >> But this is just a feature test macro in a configure script. >> >> What about the code itself, how do we handle this when we >> actually use preadv() in the codebase? > > Its not clear to me what the issue would be by defining this extra > _DEFAULT_SOURCE when _BSD_SOURCE is defined. > That's not the issue I was raising; I'm fine with defining both. I pointed out the man page because it clearly describes the change. What I'm asking is this: one or the other of these defines is /required/ for preadv use, per the man page. You are fixing one in the config test. What about the actual use of preadv in the code itself? Why does the issue exist only in the config test? Eric > Luis > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 2, 2017 at 3:51 PM, Eric Sandeen <sandeen@sandeen.net> wrote: > On Nov 2, 2017, at 3:22 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: >> >>> On Thu, Nov 2, 2017 at 1:04 PM, Eric Sandeen <sandeen@sandeen.net> wrote: >>>> On 10/26/17 12:28 PM, Luis R. Rodriguez wrote: >>>> ./configure will leave traces of a complaint on config.log when >>>> _BSD_SOURCE is defined but not _DEFAULT_SOURCE. _BSD_SOURCE is >>>> deprecated so if defining _BSD_SOURCE also define _DEFAULT_SOURCE. >>>> >>>> From config.log: >>>> /usr/include/features.h:183:3: warning: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Wcpp] >>> >>> Ok. Or perhaps a more clear rationale is the preadv(5) manpage >>> itself: >>> >>> preadv(), pwritev(): >>> since glibc 2.19: >>> _DEFAULT_SOURCE >>> Glibc 2.19 and earlier: >>> _BSD_SOURCE >>> >>> But this is just a feature test macro in a configure script. >>> >>> What about the code itself, how do we handle this when we >>> actually use preadv() in the codebase? >> >> Its not clear to me what the issue would be by defining this extra >> _DEFAULT_SOURCE when _BSD_SOURCE is defined. >> > That's not the issue I was raising; I'm fine with defining both. I pointed out the man page because it clearly describes the change. > > What I'm asking is this: one or the other of these defines is /required/ for preadv use, per the man page. You are fixing one in the config test. What about the actual use of preadv in the code itself? Why does the issue exist only in the config test? Beats me really, I ran into this pesky warning with modern compilers and on old releases, and on old releases the complaint is actually on most of the C files it compiles, with new code for some reason it only complains on the configuration. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/2/17 5:59 PM, Luis R. Rodriguez wrote: > On Thu, Nov 2, 2017 at 3:51 PM, Eric Sandeen <sandeen@sandeen.net> wrote: >> On Nov 2, 2017, at 3:22 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: >>> >>>> On Thu, Nov 2, 2017 at 1:04 PM, Eric Sandeen <sandeen@sandeen.net> wrote: >>>>> On 10/26/17 12:28 PM, Luis R. Rodriguez wrote: >>>>> ./configure will leave traces of a complaint on config.log when >>>>> _BSD_SOURCE is defined but not _DEFAULT_SOURCE. _BSD_SOURCE is >>>>> deprecated so if defining _BSD_SOURCE also define _DEFAULT_SOURCE. >>>>> >>>>> From config.log: >>>>> /usr/include/features.h:183:3: warning: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Wcpp] >>>> >>>> Ok. Or perhaps a more clear rationale is the preadv(5) manpage >>>> itself: >>>> >>>> preadv(), pwritev(): >>>> since glibc 2.19: >>>> _DEFAULT_SOURCE >>>> Glibc 2.19 and earlier: >>>> _BSD_SOURCE >>>> >>>> But this is just a feature test macro in a configure script. >>>> >>>> What about the code itself, how do we handle this when we >>>> actually use preadv() in the codebase? >>> >>> Its not clear to me what the issue would be by defining this extra >>> _DEFAULT_SOURCE when _BSD_SOURCE is defined. >>> >> That's not the issue I was raising; I'm fine with defining both. I pointed out the man page because it clearly describes the change. >> >> What I'm asking is this: one or the other of these defines is /required/ for preadv use, per the man page. You are fixing one in the config test. What about the actual use of preadv in the code itself? Why does the issue exist only in the config test? > > Beats me really, I ran into this pesky warning with modern compilers > and on old releases, and on old releases the complaint is actually on > most of the C files it compiles, with new code for some reason it only > complains on the configuration. I'm asking you to look into it, like this: The config snippet is simply a tiny bit of C code, generated from the m4: #define _BSD_SOURCE #include <sys/uio.h> int main () { preadv(0, 0, 0, 0); ; return 0; } you're telling me that _BSD_SOURCE is deprecated and should now be _DEFAULT_SOURCE - which is fine. The preadv manpage says one or the other is required, to get the definition. Defining both is fine, for older glibcs. In fact this is what the feature_test_macros(7) manpage says to do. But the whole point of a config test for preadv is so that we can /use/ it in the code, right? So surely if this were a problem in the preadv /test/ it is likely to be a problem in the preadv /use/ within the code as well, right? In other words, don't just paper over a warning with a spot-fix; look at what the warning is telling you, and fix things across the project in a consistent manner. preadv() is called in io/pread.c - if the config test passes. Oddly, it doesn't use either of the #defines before including <sys/uio.h>, despite the manpage listing this as a requirement. Apparently it's a happy accident that it still works. So now that the code in configure has highlighted this problem for you, look at everywhere preadv and/or _BSD_SOURCE is used, and add the proper #defines per the manpage there as well. If you can send a V2 that also adds proper #defines for the preadv use in io/pread.c, I'd appreciate it. Thanks, -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4 index fa5b63978797..af2a9631b8ba 100644 --- a/m4/package_libcdev.m4 +++ b/m4/package_libcdev.m4 @@ -137,6 +137,7 @@ AC_DEFUN([AC_HAVE_PREADV], [ AC_MSG_CHECKING([for preadv]) AC_TRY_LINK([ #define _BSD_SOURCE +#define _DEFAULT_SOURCE #include <sys/uio.h> ], [ preadv(0, 0, 0, 0);
./configure will leave traces of a complaint on config.log when _BSD_SOURCE is defined but not _DEFAULT_SOURCE. _BSD_SOURCE is deprecated so if defining _BSD_SOURCE also define _DEFAULT_SOURCE. From config.log: /usr/include/features.h:183:3: warning: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Wcpp] Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> --- m4/package_libcdev.m4 | 1 + 1 file changed, 1 insertion(+)