Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
An example is probably the simplest option. I'll create a fork and share the changelist with you. - Eric On Wed, Jun 3, 2020 at 6:12 AM Rob Landley wrote: > On 6/2/20 4:13 PM, Eric Molitor wrote: > > Mmh, I just switched my dev machine from Alpine to Ubuntu before working > on > > this, normally I would have caught this. > > > > Rob, what are your thoughts on using GitHub actions to build toybox on > Alpine, > > Ubuntu and MacOS and running through the test suite? This would have > caught this > > earlier. I'm happy to submit a patch to set this up if it is desired. > > In theory you could do this through your own fork/branch if you want. > > I'm reluctant to become dependant on microsoft github for any actual > functions. > I'd rather set up a build test here. (That's part of what the mkroot stuff > is > for, I noticed because it broke the mkroot build using the mcm toolchain.) > > What microsoft github can do that my setup can't is test under Apple MacOS, > because that's proprietary and running it under qemu is not allowed for > intellectual property reasons. > > In theory you can kind of hack it: > > https://lists.gnu.org/archive/html/qemu-devel/2008-02/msg00444.html > > In practice, I've never bothered trying. (And I haven't seen a retail > boxed copy > of macos at Fry's in years anyway.) > > Hmmm... What would the patch to do it look like? (I dunno how you trigger > github > actions...) > > Rob > ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
On Wed, Jun 03, 2020 at 08:49:47AM -0500, Rob Landley wrote:
> On 6/2/20 12:53 PM, Rich Felker wrote:
> > On Tue, Jun 02, 2020 at 12:42:41PM -0500, Rob Landley wrote:
> >> On 6/2/20 12:25 PM, Rich Felker wrote:
> >>> This likely needs a new __UAPI_* macro to suppress it. It's likely
> >>> that nobody has hit it because sys/sysinfo.h is a pretty obscure
> >>> header and it's usually not included anywhere except uptime(1), etc.
> >>> Having it in lib/portability.h, rather than just in the files that
> >>> need it, is probably not a good idea.
> >>
> >> It built fine with glibc, which is why it got committed unnoticed.
> >>
> >>> I'm not sure how kernel folks will want to fix this, if at all. Once
> >>> we get an idea of that I can include a patch in mcm for the kernel
> >>> header that matches what upstream is doing.
> >>
> >> Again, builds fine with glibc. I commited a workaround for the musl bug.
> >
> > This is not a musl bug. I'm really really REALLY tired of you calling
> > things musl bugs when they obviously are not.
>
> Sorry. I'm _really_ stressed right now.
Apology accepted.
> For context, I'm frustrated that musl-cross-make seems to be reproducing the
> "forked kernel header" era that Maszur did 15 years ago. Remember when Linux
> From Scratch tried to put together a FAQ about it?
This very intentionally is not what I'm doing. Use of the sabotage
linux-headers package is merely an optimization, and intended to be
functionally equivalent to the upstream headers. If there are actually
functional changes between them please let me know so I can try to get
it resolved. Because I really really don't want to go that direction.
> That was the context within which I hit _this_ problem, which is another
> aspect
> of musl's unique refusal to #include kernel headers. Both glibc and bionic do
> so
> here, bionic's sys/sysinfo.h has:
>
> #include
> #include
>
> I.E. This ONLY breaks on musl. My undestanding of your position on the issue
> is
> "that's not a bug, that's a feature". Which is another way of saying it's a
> choice you made that broke stuff.
musl does not use kernel headers. It does not require them to build.
This is a standard and completely reasonable policy. It was also
supposed to be glibc policy from the early days (one of the big
improvements over libc5) but they kept randomly breaking it and
locking themselves into instances of breaking it.
In the specific case of sysinfo, we *can't* use the kernel header
because the type does not match. Not for x32 anyway. At the time x32
was created, glibc made the (possibly unconscious) decision to ignore
existing specifications of types, in both standard and linux-specific
interfaces, and replace long with __syscall_slong_t, etc. to match the
x86_64 types. struct sysinfo is such an interface. Note that if you
type "man sysinfo", you get:
struct sysinfo {
long uptime; /* Seconds since boot */
...
This means applications can write:
struct sysinfo si;
sysinfo();
printf("%ld", si.uptime);
But on glibc x32 which uses the x86_64 definition of sysinfo, this is
a type mismatch. (In practice it might not break with printf anyway
but it will break hard in other examples.)
This kind of x32-specific breakage was probably (maybe a small) part
of why nobody ever used x32 anyway. But on musl the work was done to
make it actually work right in hopes that x32 would be useful. The
musl struct sysinfo in sys/sysinfo.h is the documented userspace API,
the sysinfo() function fills it in to match, and any program that's
using the documented API works just as well on x32 as elsewhere.
> So I added a workaround for another unique idiosyncrasy in a project that made
> the also unique choice not to #define _MUSL_.
I don't get why you're repeatedly bothered by the fact that musl did
not do everything the same way as glibc. If we had done so there would
be no point in having musl. The whole premise is that there were a lot
of things in glibc that could/should have done better. Some of them
are fixable and glibc gradually adopts improved approaches from musl
where they are, and where there's consensus that the way musl is doing
it is better. In other instances, they remain different, and *that's
ok*.
Rich
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
On 6/2/20 12:53 PM, Rich Felker wrote: > On Tue, Jun 02, 2020 at 12:42:41PM -0500, Rob Landley wrote: >> On 6/2/20 12:25 PM, Rich Felker wrote: >>> This likely needs a new __UAPI_* macro to suppress it. It's likely >>> that nobody has hit it because sys/sysinfo.h is a pretty obscure >>> header and it's usually not included anywhere except uptime(1), etc. >>> Having it in lib/portability.h, rather than just in the files that >>> need it, is probably not a good idea. >> >> It built fine with glibc, which is why it got committed unnoticed. >> >>> I'm not sure how kernel folks will want to fix this, if at all. Once >>> we get an idea of that I can include a patch in mcm for the kernel >>> header that matches what upstream is doing. >> >> Again, builds fine with glibc. I commited a workaround for the musl bug. > > This is not a musl bug. I'm really really REALLY tired of you calling > things musl bugs when they obviously are not. Sorry. I'm _really_ stressed right now. For context, I'm frustrated that musl-cross-make seems to be reproducing the "forked kernel header" era that Maszur did 15 years ago. Remember when Linux >From Scratch tried to put together a FAQ about it? http://lists.linuxfromscratch.org/pipermail/faq/2004-July/000159.html I had significant problems with it at the time: https://lore.kernel.org/lkml/200412041949.57466@landley.net/ https://lore.kernel.org/lkml/200412051805.20980@landley.net/ And checking my old blog entries, I'm reminded that the REASON it got so bad is everybody was using 2.4 headers all through 2.5: https://landley.livejournal.com/766.html https://landley.livejournal.com/8418.html And after 2.6 shipped and people started moving their libc builds to 2.6 headers they found All The Regressions and the kernel guys went "you haven't bothered us about this for YEARS why are you bitching now" and they went "because 2.5 wasn't usable" and this whole thing is part of the reason the kernel stopped doing even/odd development cycles and instead switched to the perpetually stable timed quarterly releases thing. The forked headers were basically just another variant of "everybody's patches are against 2.4, we need to forward port stuff and Dave Jones has a tree backporting stuff to and it's a mess re-unifying and finally getting everybody to trust 2.6" (which took about 2 years if I recall). I had a nontrivial role in ENDING the need for the Maszur header fork and getting proper header support upstream resolved (David possibly Woodward did the heavy lifting, but I was an early adopter testing and sending feedback and bug reports and getting it to work in uClibc and such), and then half my perl removal patches were rewriting the make headers_install infrastructure from scratch so a good chunk of the code that's there to do it now _I_ wrote, and I still get the occasional cc: on patches to it. Now you're returning musl-cross-make to the days of forked headers that need to be independently debugged from upstream. I noticed this time because those headers have nothing for m68k and s390x, so they broke my toolchains. You accepted notification of that bug back in March: http://lists.landley.net/pipermail/toybox-landley.net/2020-March/011536.html But I just pulled musl-cross-make (last updated in April) and the LINUX_VER line in the makefile still git annotates to: 38e52db8 (Rich Felker 2019-12-18 14:29:07 -0500 11)LINUX_VER = headers-4.19.88 (The s390x duplicate dynamic linker definition's still there too.) That was the context within which I hit _this_ problem, which is another aspect of musl's unique refusal to #include kernel headers. Both glibc and bionic do so here, bionic's sys/sysinfo.h has: #include #include I.E. This ONLY breaks on musl. My undestanding of your position on the issue is "that's not a bug, that's a feature". Which is another way of saying it's a choice you made that broke stuff. So I added a workaround for another unique idiosyncrasy in a project that made the also unique choice not to #define _MUSL_. > Rich Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
On 6/2/20 4:13 PM, Eric Molitor wrote: > Mmh, I just switched my dev machine from Alpine to Ubuntu before working on > this, normally I would have caught this. > > Rob, what are your thoughts on using GitHub actions to build toybox on Alpine, > Ubuntu and MacOS and running through the test suite? This would have caught > this > earlier. I'm happy to submit a patch to set this up if it is desired. In theory you could do this through your own fork/branch if you want. I'm reluctant to become dependant on microsoft github for any actual functions. I'd rather set up a build test here. (That's part of what the mkroot stuff is for, I noticed because it broke the mkroot build using the mcm toolchain.) What microsoft github can do that my setup can't is test under Apple MacOS, because that's proprietary and running it under qemu is not allowed for intellectual property reasons. In theory you can kind of hack it: https://lists.gnu.org/archive/html/qemu-devel/2008-02/msg00444.html In practice, I've never bothered trying. (And I haven't seen a retail boxed copy of macos at Fry's in years anyway.) Hmmm... What would the patch to do it look like? (I dunno how you trigger github actions...) Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
Mmh, I just switched my dev machine from Alpine to Ubuntu before working on this, normally I would have caught this. Rob, what are your thoughts on using GitHub actions to build toybox on Alpine, Ubuntu and MacOS and running through the test suite? This would have caught this earlier. I'm happy to submit a patch to set this up if it is desired. - Eric On Tue, Jun 2, 2020 at 7:48 PM Rich Felker wrote: > On Tue, Jun 02, 2020 at 12:42:41PM -0500, Rob Landley wrote: > > On 6/2/20 12:25 PM, Rich Felker wrote: > > > This likely needs a new __UAPI_* macro to suppress it. It's likely > > > that nobody has hit it because sys/sysinfo.h is a pretty obscure > > > header and it's usually not included anywhere except uptime(1), etc. > > > Having it in lib/portability.h, rather than just in the files that > > > need it, is probably not a good idea. > > > > It built fine with glibc, which is why it got committed unnoticed. > > To clarify here, I was talking about why the kernel header clash went > unnotice for so long in the entire ecosysten, not in toybox. There's > never been a report of this before, and it's likely because there are > only 6 kernel headers which include the offending linux/kernel.h, and > none of them are things that would typically be included in the same > file using sysinfo(). This is among tens of thousands of packages. > > That's not to say it's wrong to include both linux/netlink.h and > sys/sysinfo.h, just an explanation of how the bug survived so long. > > > > I'm not sure how kernel folks will want to fix this, if at all. Once > > > we get an idea of that I can include a patch in mcm for the kernel > > > header that matches what upstream is doing. > > > > Again, builds fine with glibc. I commited a workaround for the musl bug. > > I checked out your workaround and it should work fine at present and > for the forseeable future. I'd still like to get this fixed properly, > but that will be a discussion with kernel folks. > > Rich > ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
On Tue, Jun 02, 2020 at 12:42:41PM -0500, Rob Landley wrote: > On 6/2/20 12:25 PM, Rich Felker wrote: > > This likely needs a new __UAPI_* macro to suppress it. It's likely > > that nobody has hit it because sys/sysinfo.h is a pretty obscure > > header and it's usually not included anywhere except uptime(1), etc. > > Having it in lib/portability.h, rather than just in the files that > > need it, is probably not a good idea. > > It built fine with glibc, which is why it got committed unnoticed. To clarify here, I was talking about why the kernel header clash went unnotice for so long in the entire ecosysten, not in toybox. There's never been a report of this before, and it's likely because there are only 6 kernel headers which include the offending linux/kernel.h, and none of them are things that would typically be included in the same file using sysinfo(). This is among tens of thousands of packages. That's not to say it's wrong to include both linux/netlink.h and sys/sysinfo.h, just an explanation of how the bug survived so long. > > I'm not sure how kernel folks will want to fix this, if at all. Once > > we get an idea of that I can include a patch in mcm for the kernel > > header that matches what upstream is doing. > > Again, builds fine with glibc. I commited a workaround for the musl bug. I checked out your workaround and it should work fine at present and for the forseeable future. I'd still like to get this fixed properly, but that will be a discussion with kernel folks. Rich ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
On Tue, Jun 02, 2020 at 12:42:41PM -0500, Rob Landley wrote: > On 6/2/20 12:25 PM, Rich Felker wrote: > > This likely needs a new __UAPI_* macro to suppress it. It's likely > > that nobody has hit it because sys/sysinfo.h is a pretty obscure > > header and it's usually not included anywhere except uptime(1), etc. > > Having it in lib/portability.h, rather than just in the files that > > need it, is probably not a good idea. > > It built fine with glibc, which is why it got committed unnoticed. > > > I'm not sure how kernel folks will want to fix this, if at all. Once > > we get an idea of that I can include a patch in mcm for the kernel > > header that matches what upstream is doing. > > Again, builds fine with glibc. I commited a workaround for the musl bug. This is not a musl bug. I'm really really REALLY tired of you calling things musl bugs when they obviously are not. Rich ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
On 6/2/20 12:25 PM, Rich Felker wrote: > This likely needs a new __UAPI_* macro to suppress it. It's likely > that nobody has hit it because sys/sysinfo.h is a pretty obscure > header and it's usually not included anywhere except uptime(1), etc. > Having it in lib/portability.h, rather than just in the files that > need it, is probably not a good idea. It built fine with glibc, which is why it got committed unnoticed. > I'm not sure how kernel folks will want to fix this, if at all. Once > we get an idea of that I can include a patch in mcm for the kernel > header that matches what upstream is doing. Again, builds fine with glibc. I commited a workaround for the musl bug. Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
On Tue, Jun 02, 2020 at 10:56:37AM -0500, Rob Landley wrote:
> On 6/1/20 10:56 AM, Eric Molitor wrote:
> > ---
> > toys/pending/route.c | 46 +++-
> > 1 file changed, 33 insertions(+), 13 deletions(-)
>
> FYI route.c doesn't build with musl-cross-make because:
>
> ~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/sys/sysinfo.h:10:8: error:
> redefinition of 'struct sysinfo'
>10 | struct sysinfo {
> |^~~
> In file included from
> ~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/linux/kernel.h:5,
> from
> ~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/linux/netlink.h:5,
> from
> ~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/linux/rtnetlink.h:6,
> from toys/pending/route.c:50:
> ~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/linux/sysinfo.h:8:8: note:
> originally defined here
>
> Builds find with glibc, but not with musl.
>
> I'm pretty sure this is because glibc does this in sys/sinfo.h:
>
> /* Get sysinfo structure from kernel header. */
> #include
>
> And if you DON'T do that, including linux/rtnetlink.h along with sys/sysinfo.h
> breaks the build.
This likely needs a new __UAPI_* macro to suppress it. It's likely
that nobody has hit it because sys/sysinfo.h is a pretty obscure
header and it's usually not included anywhere except uptime(1), etc.
Having it in lib/portability.h, rather than just in the files that
need it, is probably not a good idea.
With that said it *also* looks wrong that linux/netlink.h is including
linux/kernel.h, when the only purpose of the latter for userspace
seems to be serving as a legacy name for linux/sysinfo.h (to make
glibc happy). linux/netlink.h is doing this to get a definition for
__ALIGN_KERNEL which it uses in excactly one place:
#define NL_MMAP_MSG_ALIGN(sz) __ALIGN_KERNEL(sz,
NL_MMAP_MSG_ALIGNMENT)
Indeed the inclusion was introduced in commit
ccdfcc398594ddf3f77348c5a10938dbe9efefbe, without any regard for the
namespace pollution.
I'm not sure how kernel folks will want to fix this, if at all. Once
we get an idea of that I can include a patch in mcm for the kernel
header that matches what upstream is doing.
Rich
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
On 6/1/20 10:56 AM, Eric Molitor wrote:
> ---
> toys/pending/route.c | 46 +++-
> 1 file changed, 33 insertions(+), 13 deletions(-)
FYI route.c doesn't build with musl-cross-make because:
~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/sys/sysinfo.h:10:8: error:
redefinition of 'struct sysinfo'
10 | struct sysinfo {
|^~~
In file included from
~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/linux/kernel.h:5,
from
~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/linux/netlink.h:5,
from
~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/linux/rtnetlink.h:6,
from toys/pending/route.c:50:
~/ccc/sh4-linux-musl-cross/sh4-linux-musl/include/linux/sysinfo.h:8:8: note:
originally defined here
Builds find with glibc, but not with musl.
I'm pretty sure this is because glibc does this in sys/sinfo.h:
/* Get sysinfo structure from kernel header. */
#include
And if you DON'T do that, including linux/rtnetlink.h along with sys/sysinfo.h
breaks the build.
Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] [PATCH 1/4] Implement hostname lookups in display_routes
---
toys/pending/route.c | 46 +++-
1 file changed, 33 insertions(+), 13 deletions(-)
diff --git a/toys/pending/route.c b/toys/pending/route.c
index 39ba160b..9c95d996 100644
--- a/toys/pending/route.c
+++ b/toys/pending/route.c
@@ -94,6 +94,18 @@ void addAttr(struct nlmsghdr *nl, int maxlen, void *attr,
int type, int len)
nl->nlmsg_len = NLMSG_ALIGN(nl->nlmsg_len) + rtlen;
}
+static void get_hostname(sa_family_t family, struct in_addr *addr, char *dst,
size_t len) {
+ struct sockaddr_in sa;
+
+ memset(, 0, sizeof sa);
+ sa.sin_family = family;
+ sa.sin_addr = *addr;
+
+ if (getnameinfo((struct sockaddr*), sizeof(sa), dst, len,NULL, 0,
NI_NOFQDN)) {
+perror_exit("getnameinfo");
+ }
+}
+
static void display_routes(sa_family_t family)
{
int fd, msg_hdr_len, route_protocol;
@@ -139,8 +151,8 @@ static void display_routes(sa_family_t family)
// have to filter here.
if (route_entry->rtm_table == RT_TABLE_MAIN) {
struct in_addr netmask_addr;
-char destip[INET6_ADDRSTRLEN];
-char gateip[INET6_ADDRSTRLEN];
+char dest[INET6_ADDRSTRLEN];
+char gate[INET6_ADDRSTRLEN];
char netmask[32];
char flags[10] = "U";
uint32_t priority = 0;
@@ -156,14 +168,14 @@ static void display_routes(sa_family_t family)
struct rta_cacheinfo *cache_info;
if (family == AF_INET) {
- if (!(toys.optflags & FLAG_n)) strcpy(destip, "default");
- else strcpy(destip, "0.0.0.0");
- if (!(toys.optflags & FLAG_n)) strcpy(gateip, "*");
- else strcpy(gateip, "0.0.0.0");
+ if (!(toys.optflags & FLAG_n)) strcpy(dest, "default");
+ else strcpy(dest, "0.0.0.0");
+ if (!(toys.optflags & FLAG_n)) strcpy(gate, "*");
+ else strcpy(gate, "0.0.0.0");
strcpy(netmask, "0.0.0.0");
} else {
- strcpy(destip, "::");
- strcpy(gateip, "::");
+ strcpy(dest, "::");
+ strcpy(gate, "::");
}
route_netmask = route_entry->rtm_dst_len;
@@ -179,11 +191,19 @@ static void display_routes(sa_family_t family)
while (RTA_OK(route_attribute, route_attribute_len)) {
switch (route_attribute->rta_type) {
case RTA_DST:
- inet_ntop(family, RTA_DATA(route_attribute), destip,
INET6_ADDRSTRLEN);
+ if (toys.optflags & FLAG_n) {
+inet_ntop(family, RTA_DATA(route_attribute), dest,
sizeof(dest));
+ } else {
+get_hostname(family, RTA_DATA(route_attribute), dest,
sizeof(dest));
+ }
break;
case RTA_GATEWAY:
- inet_ntop(family, RTA_DATA(route_attribute), gateip,
INET6_ADDRSTRLEN);
+ if (toys.optflags & FLAG_n) {
+inet_ntop(family, RTA_DATA(route_attribute), gate,
sizeof(dest));
+ } else {
+get_hostname(family, RTA_DATA(route_attribute), gate,
sizeof(dest));
+ }
strcat(flags, "G");
break;
@@ -225,14 +245,14 @@ static void display_routes(sa_family_t family)
if (route_protocol == RTPROT_REDIRECT) strcat(flags, "D");
if (family == AF_INET) {
- xprintf("%-15.15s %-15.15s %-16s%-6s", destip, gateip, netmask,
flags);
+ xprintf("%-15.15s %-15.15s %-16s%-6s", dest, gate, netmask, flags);
if (toys.optflags & FLAG_e) {
xprintf("%5d %-5d %6d %s\n", mss, win, irtt, if_name);
} else xprintf("%-6d %-2d %7d %s\n", priority, ref, use, if_name);
} else {
- char *dest_with_mask = xmprintf("%s/%u", destip, route_netmask);
+ char *dest_with_mask = xmprintf("%s/%u", dest, route_netmask);
xprintf("%-30s %-26s %-4s %-6d %-4d %2d %-8s\n",
- dest_with_mask, gateip, flags, priority, ref, use, if_name);
+ dest_with_mask, gate, flags, priority, ref, use, if_name);
free(dest_with_mask);
}
}
--
2.25.1
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
