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).
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...:
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?
Obviously it could be '#if !defined MUSL' instead...
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.
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
which is in aosp to this day... see:
https://android.git.corp.google.com/platform/external/iptables/+/refs/heads/master/libxtables/xtables.c#48
If I revert that (ie. remove the above 3 lines), then aosp compilation fails:
external/iptables/libxtables/xtables.c:2144:45: error: use of
undeclared identifier 'ETH_ALEN'
static const unsigned char mac_type_unicast[ETH_ALEN] = {};
^
...etc...
which suggests the "#include <netinet/ether.h>" immediately before
that isn't sufficient.
I think that should include
https://cs.android.com/android/platform/superproject/+/master:bionic/libc/include/netinet/ether.h
which should #include <netinet/if_ether.h> which is
https://cs.android.com/android/platform/superproject/+/master:bionic/libc/include/netinet/if_ether.h
which should #include <linux/if_ether.h>
but... only #if defined(__USE_BSD)
is the problem perhaps the lack of __USE_BSD?
And indeed defining __USE_BSD just before the include:
+#define __USE_BSD 1
#include <netinet/ether.h>
fixes the build on aosp (with the 3 lines still removed).
But I have no idea if we should or should not #define __USE_BSD ...
And who / what is actually wrong...
