Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
On 2018-10-01, David Phillips wrote: > On Mon, Oct 01, 2018 at 06:35:13PM -0700, Michael Forney wrote: >> Rather than the if-else, I think we should always include sys/types.h, >> and also sys/sysmacros.h on glibc. I also think the comment is >> unnecessary. Do you mind if I apply with those two changes? > > I agree with that logic, so long as that doesn't hurt BSD systems (I > can't recall at the moment if sys/types.h exists on BSD) Yes, sys/types.h is POSIX, so all systems will have it. I applied with the changes I mentioned. Thanks for the patch!
Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
On 2018-10-01, David Phillips wrote: > Hi, > > Bumping this patch since sbase master now fails to build against > glibc 2.28 > > Let me know if there are any improvements that could be made. > > Thanks, > David I wish the #ifdef wasn't necessary, but it seems like the only way to use major/minor on glibc and non-glibc systems. As far as I can tell, glibc has had sys/sysmacros.h for a long time, so it should be okay to always include it when __GLIBC__ is defined. Rather than the if-else, I think we should always include sys/types.h, and also sys/sysmacros.h on glibc. I also think the comment is unnecessary. Do you mind if I apply with those two changes?
Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
On Mon, Jul 02, 2018 at 11:58:54PM +1200, David Phillips wrote: > On Mon, Jul 02, 2018 at 01:20:42PM +0200, Quentin Rameau wrote: > > Ok, the makedev(3) manpage from the man-pages states this indeed: > > > > The BSDs expose the definitions for these macros via . > > Depending on the version, glibc also exposes definitions for these > > macros from that header file if suitable feature test macros are > > defined. However, this behavior was deprecated in glibc 2.25, and since > > glibc 2.28, no longer provides these definitions. > > > > musl still includes from so maybe we > > would only need an #ifdef __GLIBC__ there… > > > > Or a more complicated > > #ifdef __GLIBC__ > > #if __GLIBC_PREREQ(2, 25) > > #include > > #endif > > #else > > #include > > #endif > > To simplify things on our side, couldn't we just include > if linking against glibc, regardless of its > version? > > It sounds like glibc has defined these in > for a while into the past (citation needed) and all that > is going to change is the inclusion of this file from types.h > > I know it's less "correct", but it doesn't seem prone to > breakage if that assumption is correct. > > Might not be worth the slight increase in clarity though, not > really sure. > > Thanks, > David Reworked patch encompassing this behaviour is attached. Would be interested to see builds against glibc pre-2.25. Let me know what you think. Thanks, David >From 331a370ffd51876a98e81ef330f27631c4e46859 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Mon, 2 Jul 2018 19:42:41 +1200 Subject: [PATCH] [sbase] On glibc, include sysmacros.h directly On glibc, major, minor, and makedev are all defined in sys/sysmacros.h with types.h only including this for historical reasons. A future release of glibc will remove this behaviour, meaning that major, minor, and makedev will no longer be defined for us without including sysmacros.h. --- ls.c | 7 +++ tar.c | 8 2 files changed, 15 insertions(+) diff --git a/ls.c b/ls.c index b716aba..4c269bd 100644 --- a/ls.c +++ b/ls.c @@ -1,6 +1,13 @@ /* See LICENSE file for copyright and license details. */ #include + +/* glibc 2.25+ deprecates bringing sysmacros.h in with types.h anymore + * to define major and minor */ +#ifdef __GLIBC__ +#include +#else #include +#endif #include #include diff --git a/tar.c b/tar.c index a6ead2e..c4a7435 100644 --- a/tar.c +++ b/tar.c @@ -2,6 +2,14 @@ #include #include +/* glibc 2.25+ deprecates bringing sysmacros.h in with types.h anymore + * to define major, minor, and makedev */ +#ifdef __GLIBC__ +#include +#else +#include +#endif + #include #include #include -- 2.17.1
Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
On Mon, Jul 02, 2018 at 01:20:42PM +0200, Quentin Rameau wrote: > Ok, the makedev(3) manpage from the man-pages states this indeed: > > The BSDs expose the definitions for these macros via . > Depending on the version, glibc also exposes definitions for these > macros from that header file if suitable feature test macros are > defined. However, this behavior was deprecated in glibc 2.25, and since > glibc 2.28, no longer provides these definitions. > > musl still includes from so maybe we > would only need an #ifdef __GLIBC__ there… > > Or a more complicated > #ifdef __GLIBC__ > #if __GLIBC_PREREQ(2, 25) > #include > #endif > #else > #include > #endif To simplify things on our side, couldn't we just include if linking against glibc, regardless of its version? It sounds like glibc has defined these in for a while into the past (citation needed) and all that is going to change is the inclusion of this file from types.h I know it's less "correct", but it doesn't seem prone to breakage if that assumption is correct. Might not be worth the slight increase in clarity though, not really sure. Thanks, David
Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
> > On glibc, major, minor, and makedev are all defined in > > sys/sysmacros.h with types.h only including this for historical > > reasons. A future release of glibc will remove this behaviour, > > meaning that major, minor, and makedev will no longer be defined > > for us without including sysmacros.h. > > Could you link to the source of that information? Ok, the makedev(3) manpage from the man-pages states this indeed: The BSDs expose the definitions for these macros via . Depending on the version, glibc also exposes definitions for these macros from that header file if suitable feature test macros are defined. However, this behavior was deprecated in glibc 2.25, and since glibc 2.28, no longer provides these definitions. musl still includes from so maybe we would only need an #ifdef __GLIBC__ there… Or a more complicated #ifdef __GLIBC__ #if __GLIBC_PREREQ(2, 25) #include #endif #else #include #endif
Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
On Mon, Jul 02, 2018 at 10:16:50AM +0200, Quentin Rameau wrote: > > On glibc, major, minor, and makedev are all defined in > > sys/sysmacros.h with types.h only including this for historical > > reasons. A future release of glibc will remove this behaviour, > > meaning that major, minor, and makedev will no longer be defined > > for us without including sysmacros.h. > > Could you link to the source of that information? > > Thanks. Sure thing. See the changelog for glibc-2.25 [1]. The manpage for these functions aas provided by my current version of glibc (2.27) says that the inclusion of sysmacros.h from types.h will be removed by 2.28 (cannot find an alternative source for that at the moment). Thanks, David [1]: https://sourceware.org/ml/libc-alpha/2017-02/msg00079.html
Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
On Mon, Jul 02, 2018 at 10:02:09AM +0200, Hiltjo Posthuma wrote: > > Did you test it with musl too and preferably other platforms? Unfortunately not; I don't currently have access to non-glibc Linux installations at the moment, nor BSDs. Perhaps users of those systems might be able to chime in at some point. Looking at some documentation, it looks like BSD will keep their definitions in types.h, so this patch may need some rework with ifdefs. Would be great to have some feedback on that one. Thanks, David
Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
> On glibc, major, minor, and makedev are all defined in > sys/sysmacros.h with types.h only including this for historical > reasons. A future release of glibc will remove this behaviour, > meaning that major, minor, and makedev will no longer be defined > for us without including sysmacros.h. Could you link to the source of that information? Thanks.
Re: [hackers] [PATCH] [sbase] Include sysmacros.h directly rather than types.h
On Mon, Jul 02, 2018 at 07:48:03PM +1200, David Phillips wrote: > On glibc, major, minor, and makedev are all defined in > sys/sysmacros.h with types.h only including this for historical > reasons. A future release of glibc will remove this behaviour, > meaning that major, minor, and makedev will no longer be defined > for us without including sysmacros.h. > --- > ls.c | 2 +- > tar.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/ls.c b/ls.c > index b716aba..884776f 100644 > --- a/ls.c > +++ b/ls.c > @@ -1,6 +1,6 @@ > /* See LICENSE file for copyright and license details. */ > #include > -#include > +#include > > #include > #include > diff --git a/tar.c b/tar.c > index a6ead2e..9359bfd 100644 > --- a/tar.c > +++ b/tar.c > @@ -1,6 +1,7 @@ > /* See LICENSE file for copyright and license details. */ > #include > #include > +#include > > #include > #include > -- > 2.17.1 > > Did you test it with musl too and preferably other platforms? -- Kind regards, Hiltjo
