search

open-iscsi / open-isns

iSNS server and client for Linux
GNU Lesser General Public License v2.1
26 stars 22 forks source link

fix include of endian.h on musl-libc based systems #11

Closed prometheanfire closed 7 years ago

prometheanfire commented 7 years ago

This probably isn't the totally correct thing to do as I can't test on bsd machines, but it does fix it for musl...

chris-se commented 7 years ago

This will break FreeBSD, yes.

Does musl define a macro to detect its libc? Then we could wrap that in different #ifdef invocations, something like

#ifdef macro_for_musl
# include <endian.h>
#else
# include <sys/endian.h>
#endif
prometheanfire commented 7 years ago

First, for the macro, aparently not (I'm somewhat new to using musl systems and am just trying to build an openstack image).

http://wiki.musl-libc.org/wiki/FAQ#Q:_why_is_there_no_MUSL_macro_.3F

They did say something about the BSDs actually including endian.h soon (and possibly even now for obsd).

01:43 <        bentley` > prometheanfire: that should work fine on openbsd, and i suspect newer versions of freebsd
01:44 <        bentley` > endian.h will be in a future revision of posix and the posix-ish systems have mostly implemented it already
01:44 <        bentley` > so maybe it should be "#ifdef __FreeBSD__ #include <sys/endian.h>" ;)

So maybe it's better to check for the BSDs (if endian.h doesn't exist) then glibc, then fall back to the default location and simple behavior that the BSDs use for musl (and for the BSDs if endian.h does exist for them).

chris-se commented 7 years ago

A yes, now I remember that FAQ about musl.

I never tested OpenBSD (as I'm not a user), I wrote the patch to support FreeBSD because Debian supports the FreeBSD kernel in a semi-official port (I'm one of the Debian maintainers of open-isns). The pure FreeBSD support (with BSD libc) was mostly a side-effect of that.

But yes, checking for __FreeBSD__ for pure FreeBSD should probably work. And if endian.h is really going to be in POSIX, this is probably the best solution.

Could you provide a new patch with this that works for you with musl, I'll be happy to test it on Linux (glibc), Hurd (glibc), GNU/kFreeBSD (glibc) and FreeBSD (BSD libc).

prometheanfire commented 7 years ago

I don't run bsd to be able to test. It may be better if you could make the patch (I only say that because you can test on those combos). My only requirement is that the default behavior be like it is now (with this hackish patch). If it helps I will have an openstack image published soon (requires disk-config, login root/provided key) at http://gentoo.osuosl.org/experimental/amd64/openstack/gentoo-openstack-amd64-hardened-musl-20170103.qcow2 it just needs to mirror...

If you still want me to attempt to patch this I can, but I'd need to bug you about testing on kFreeBSD and FreeBSD (and even Hurd, didn't know this supported it :P)

chris-se commented 7 years ago

Ok, I'll come back to you on that, but you'll need to test on musl, because I don't have an OpenStack setup here.

chris-se commented 7 years ago

The following patch is builds fine on the systems I've previously tested this with: Linux, GNU/kFreeBSD, Hurd (all glibc) and FreeBSD (with BSD libc).

https://gist.github.com/chris-se/07583f5230578b0791a8514d313b4677

If that works for you on your musl system, I would recommend to force-push this to this branch to update the pull request.

prometheanfire commented 7 years ago

Ok, tested on musl, thanks for the patch. Works everywhere now :D