Skip to content

Draft: Use native htonll/ntohll if available

Ryan Carsten Schmidt requested to merge ryandesign-guest/fakeroot:ryandesign-htonll into master

Recent versions of macOS (10.10 and later) have htonll and ntohll and fakeroot 1.29 (and previous versions) redefines them:

In file included from faked.c:96:
In file included from ./communicate.h:168:
./message.h:32:10: warning: 'htonll' macro redefined [-Wmacro-redefined]
# define htonll(n)  (n)
         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/sys/_endian.h:141:9: note: previous definition is here
#define htonll(x)       __DARWIN_OSSwapInt64(x)
        ^
In file included from faked.c:96:
In file included from ./communicate.h:168:
./message.h:33:10: warning: 'ntohll' macro redefined [-Wmacro-redefined]
# define ntohll(n)  (n)
         ^
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/sys/_endian.h:140:9: note: previous definition is here
#define ntohll(x)       __DARWIN_OSSwapInt64(x)
        ^

I don't know the consequence of doing that but it seemed like probably not the best thing to do so I used the change in this merge request to stop it from doing that.

This works for me on macOS 10.15 but I'm concerned that this change may cause problems for other operating systems, with which I'm not familiar. For example, macOS implements htonll and ntohll in its SDK headers using #define, so checking for them with defined() works. But I wonder if other operating systems might define them as real functions, in which case defined() would not be the right check.

I also hesitate because I worry there was some reason why it wasn't done this way when the code was originally written in 02a55ea9 or when it was changed in a7113ebf to only use an existing htonll/ntohll on Solaris. Was there some OS that has a broken htonll/ntohll that you were trying to avoid?

Maybe the safest thing to do is write a configure test that checks if htonll/ntohll exist and work and uses that result to decide whether to define your own implementations.

Merge request reports