mailing list of musl libc
 help / color / mirror / code / Atom feed
* [PATCH] uapi: Prevent redefinition of struct iphdr
@ 2019-12-22  6:02 A. Wilcox
  2019-12-26  0:34 ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: A. Wilcox @ 2019-12-22  6:02 UTC (permalink / raw)
  To: netdev, linux-api, musl; +Cc: A. Wilcox

As with struct ethhdr, the musl libc provides its own definition of the
iphdr struct.  This guard ensures software like net-tools builds correctly
on the musl libc.

The __UAPI_DEF_IPHDR definition is in ip.h itself to prevent the issue in
commit da360299b673 ("uapi/if_ether.h: move __UAPI_DEF_ETHHDR libc define")
from being seen here.

Signed-off-by: A. Wilcox <AWilcox@Wilcox-Tech.com>
---
 include/uapi/linux/ip.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
index e42d13b55cf3..d34a0d295672 100644
--- a/include/uapi/linux/ip.h
+++ b/include/uapi/linux/ip.h
@@ -83,6 +83,13 @@
 
 #define IPV4_BEET_PHMAXLEN 8
 
+/* Allow libcs to deactivate this - musl has its own copy in <netinet/ip.h> */
+
+#ifndef __UAPI_DEF_IPHDR
+#define __UAPI_DEF_IPHDR	1
+#endif
+
+#if __UAPI_DEF_IPHDR
 struct iphdr {
 #if defined(__LITTLE_ENDIAN_BITFIELD)
 	__u8	ihl:4,
@@ -104,6 +111,7 @@ struct iphdr {
 	__be32	daddr;
 	/*The options start here. */
 };
+#endif
 
 
 struct ip_auth_hdr {
-- 
2.22.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] uapi: Prevent redefinition of struct iphdr
  2019-12-22  6:02 [PATCH] uapi: Prevent redefinition of struct iphdr A. Wilcox
@ 2019-12-26  0:34 ` David Miller
  2019-12-26  1:05   ` Rich Felker
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-12-26  0:34 UTC (permalink / raw)
  To: AWilcox; +Cc: netdev, linux-api, musl

From: "A. Wilcox" <AWilcox@Wilcox-Tech.com>
Date: Sun, 22 Dec 2019 00:02:27 -0600

> @@ -83,6 +83,13 @@
>  
>  #define IPV4_BEET_PHMAXLEN 8
>  
> +/* Allow libcs to deactivate this - musl has its own copy in <netinet/ip.h> */
> +
> +#ifndef __UAPI_DEF_IPHDR
> +#define __UAPI_DEF_IPHDR	1
> +#endif

How is this a musl-only problem?  I see that glibc also defines struct iphdr
in netinet/ip.h, so why doesn't it also suffer from this?

I find it really strange that this, therefore, only happens for musl
and we haven't had thousands of reports of this conflict with glibc
over the years.

I want an explanation, and suitably appropriate adjustments to the commit
message and comments of this change.

Thank you.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: [PATCH] uapi: Prevent redefinition of struct iphdr
  2019-12-26  0:34 ` David Miller
@ 2019-12-26  1:05   ` Rich Felker
  2019-12-26  3:49     ` [musl] " David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Rich Felker @ 2019-12-26  1:05 UTC (permalink / raw)
  To: David Miller; +Cc: AWilcox, netdev, linux-api, musl

On Wed, Dec 25, 2019 at 04:34:11PM -0800, David Miller wrote:
> From: "A. Wilcox" <AWilcox@Wilcox-Tech.com>
> Date: Sun, 22 Dec 2019 00:02:27 -0600
> 
> > @@ -83,6 +83,13 @@
> >  
> >  #define IPV4_BEET_PHMAXLEN 8
> >  
> > +/* Allow libcs to deactivate this - musl has its own copy in <netinet/ip.h> */
> > +
> > +#ifndef __UAPI_DEF_IPHDR
> > +#define __UAPI_DEF_IPHDR	1
> > +#endif
> 
> How is this a musl-only problem?

I don't think it is, unless glibc's includes linux/ip.h to get the
definition, which does not seem to be the case -- at least not on the
Debian system I had handy to check on.

> I see that glibc also defines struct iphdr
> in netinet/ip.h, so why doesn't it also suffer from this?

Maybe it does.

> I find it really strange that this, therefore, only happens for musl
> and we haven't had thousands of reports of this conflict with glibc
> over the years.

It's possible that there's software that's including just one of the
headers conditional on __GLIBC__, and including both otherwise, or
something like that. Arguably this should be considered unsupported
usage; there are plenty of headers where that doesn't work and
shouldn't be expected to.

> I want an explanation, and suitably appropriate adjustments to the commit
> message and comments of this change.

Agreed. Commit messages should not imply that something is a
musl-specific workaround when it's generally the right thing to do.

Rich


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [musl] Re: [PATCH] uapi: Prevent redefinition of struct iphdr
  2019-12-26  1:05   ` Rich Felker
@ 2019-12-26  3:49     ` David Miller
  2019-12-26 11:13       ` Daniel Kolesa
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-12-26  3:49 UTC (permalink / raw)
  To: dalias; +Cc: AWilcox, netdev, linux-api, musl

From: Rich Felker <dalias@libc.org>
Date: Wed, 25 Dec 2019 20:05:15 -0500

> On Wed, Dec 25, 2019 at 04:34:11PM -0800, David Miller wrote:
>> I find it really strange that this, therefore, only happens for musl
>> and we haven't had thousands of reports of this conflict with glibc
>> over the years.
> 
> It's possible that there's software that's including just one of the
> headers conditional on __GLIBC__, and including both otherwise, or
> something like that. Arguably this should be considered unsupported
> usage; there are plenty of headers where that doesn't work and
> shouldn't be expected to.

I don't buy that, this is waaaaaay too common a header to use.

Please investigate.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [musl] Re: [PATCH] uapi: Prevent redefinition of struct iphdr
  2019-12-26  3:49     ` [musl] " David Miller
@ 2019-12-26 11:13       ` Daniel Kolesa
  2019-12-30 17:12         ` Rich Felker
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Kolesa @ 2019-12-26 11:13 UTC (permalink / raw)
  To: David Miller, musl, dalias; +Cc: AWilcox, netdev, linux-api

On Thu, Dec 26, 2019, at 04:49, David Miller wrote:
> From: Rich Felker <dalias@libc.org>
> Date: Wed, 25 Dec 2019 20:05:15 -0500
> 
> > On Wed, Dec 25, 2019 at 04:34:11PM -0800, David Miller wrote:
> >> I find it really strange that this, therefore, only happens for musl
> >> and we haven't had thousands of reports of this conflict with glibc
> >> over the years.
> > 
> > It's possible that there's software that's including just one of the
> > headers conditional on __GLIBC__, and including both otherwise, or
> > something like that. Arguably this should be considered unsupported
> > usage; there are plenty of headers where that doesn't work and
> > shouldn't be expected to.
> 
> I don't buy that, this is waaaaaay too common a header to use.

In case of net-tools, only <linux/ip.h> is included, and never <netinet/ip.h> directly. Chances are in musl the indirect include tree happens to be different and conflicting, while in glibc it is not.

> 
> Please investigate.
>

Daniel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Re: [PATCH] uapi: Prevent redefinition of struct iphdr
  2019-12-26 11:13       ` Daniel Kolesa
@ 2019-12-30 17:12         ` Rich Felker
  0 siblings, 0 replies; 6+ messages in thread
From: Rich Felker @ 2019-12-30 17:12 UTC (permalink / raw)
  To: Daniel Kolesa; +Cc: David Miller, musl, AWilcox, netdev, linux-api

On Thu, Dec 26, 2019 at 12:13:37PM +0100, Daniel Kolesa wrote:
> On Thu, Dec 26, 2019, at 04:49, David Miller wrote:
> > From: Rich Felker <dalias@libc.org>
> > Date: Wed, 25 Dec 2019 20:05:15 -0500
> > 
> > > On Wed, Dec 25, 2019 at 04:34:11PM -0800, David Miller wrote:
> > >> I find it really strange that this, therefore, only happens for musl
> > >> and we haven't had thousands of reports of this conflict with glibc
> > >> over the years.
> > > 
> > > It's possible that there's software that's including just one of the
> > > headers conditional on __GLIBC__, and including both otherwise, or
> > > something like that. Arguably this should be considered unsupported
> > > usage; there are plenty of headers where that doesn't work and
> > > shouldn't be expected to.
> > 
> > I don't buy that, this is waaaaaay too common a header to use.
> 
> In case of net-tools, only <linux/ip.h> is included, and never
> <netinet/ip.h> directly. Chances are in musl the indirect include
> tree happens to be different and conflicting, while in glibc it is
> not.

musl has no indirect inclusion of netinet/ip.h from standard headers,
but does include it from netinet/ip_icmp.h. It seems glibc only does
this conditional on __USE_MISC, which doesn't make much sense to me
since this is not a standardized header with namespace rules, but
normally __USE_MISC is defined anyway on glibc so I kinda doubt this
is the difference.

Any other ideas?

Rich


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-12-30 17:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-22  6:02 [PATCH] uapi: Prevent redefinition of struct iphdr A. Wilcox
2019-12-26  0:34 ` David Miller
2019-12-26  1:05   ` Rich Felker
2019-12-26  3:49     ` [musl] " David Miller
2019-12-26 11:13       ` Daniel Kolesa
2019-12-30 17:12         ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).