On Sun, May 15, 2022 at 5:05 AM Phil Sutter <phil@xxxxxx> wrote:
>
> Hi Maciej,
>
> On Sat, May 14, 2022 at 12:14:27PM -0700, Maciej Żenczykowski wrote:
> > On Sat, May 14, 2022 at 10:04 AM Phil Sutter <phil@xxxxxx> wrote:
> > > On Sat, May 14, 2022 at 06:33:24PM +0200, Nick Hainke wrote:
> > > > Only include <linux/if_ether.h> if glibc is used.
> > >
> > > This looks like a bug in musl? OTOH explicit include of linux/if_ether.h
> > > was added in commit c5d9a723b5159 ("fix build for missing ETH_ALEN
> > > definition"), despite netinet/ether.h being included in line 2248 of
> > > libxtables/xtables.c. So maybe *also* a bug in bionic?!
> >
> > You stripped the email you're replying to, and while I'm on lkml and
> > netdev - with my personal account - I'm not (apparently) subscribed to
> > netfilter-devel (or I'm not subscribed from work account).
>
> Oh, sorry for the caused inconvenience.
>
> > Either way, if my search-fu is correct you're replying to
> > https://marc.info/?l=netfilter-devel&m=165254651011397&w=2
> >
> > +#if defined(__GLIBC__)
> > #include <linux/if_ether.h> /* ETH_ALEN */
> > +#endif
> >
> > and you're presumably CC'ing me due to
> >
> > https://git.netfilter.org/iptables/commit/libxtables/xtables.c?id=c5d9a723b5159a28f547b577711787295a14fd84
> >
> > which added the include in the first place...:
>
> That's correct. I assumed that you added the include for a reason and
> it's breaking Nick's use-case, the two of you want to have a word with
> each other. :)
>
> > fix build for missing ETH_ALEN definition
> > (this is needed at least with bionic)
> >
> > +#include <linux/if_ether.h> /* ETH_ALEN */
> >
> > Based on the above, clearly adding an 'if defined GLIBC' wrapper will
> > break bionic...
> > and presumably glibc doesn't care whether the #include is done one way
> > or the other?
>
> With glibc, netinet/ether.h includes netinet/if_ether.h which in turn
> includes linux/if_ether.h where finally ETH_ALEN is defined.
>
> In xtables.c we definitely need netinet/ether.h for ether_aton()
> declaration.
>
> > Obviously it could be '#if !defined MUSL' instead...
>
> Could ...
>
> > As for the fix? And whether glibc or musl or bionic are wrong or not...
> > Utterly uncertain...
> >
> > Though, I will point out #include's 2000 lines into a .c file are kind of funky.
>
> ACK!
>
> > Ultimately I find
> > https://android.git.corp.google.com/platform/external/iptables/+/7608e136bd495fe734ad18a6897dd4425e1a633b%5E%21/
> >
> > +#ifdef __BIONIC__
> > +#include <linux/if_ether.h> /* ETH_ALEN */
> > +#endif
>
> While I think musl not catching the "double" include is a bug, I'd
> prefer the ifdef __BIONIC__ solution since it started the "but my libc
> needs this" game.
>
> Nick, if the above change fixes musl builds for you, would you mind
> submitting it formally along with a move of the netinet/ether.h include
> from mid-file to top?
>
> Thanks, Phil
Any thoughts about the rest of my email - wrt. #define __USE_BSD
- do you know how that is supposed to work?
