Add in missing endianess functions for Apple targets

[palmerc]

Apple SDKs indeed have the endian.h headers, but they do not contain these functions and therefore will error out when doing a cmake build. Remove the conditional HAVE_ENDIAN check or remove the conditional check for Apple compatibility? I chose to remove the conditional check for Apple compatibility.

[botovq]

Thanks. Something similar came up here #1025 but this sort of got stuck since nobody could tell me what the correct approach was. Can you tell me if this is good enough to build on iOS as well, i.e., does this address the issue in #1025?

[palmerc]

I am building macOS, iPhoneOS and iPhoneSimulator in a single shot. It is the same issue as appears in 1025

[palmerc]

A better approach might be to check for the individual functions, but it is sort of an all-or-nothing affair since none of these functions appear in AppleVerse

[botovq]

This endian.h compat thing is just such an unbelievable mess :)

We copied this from openiked, which has always had this HAVE_ENDIAN_H define.

Is it aversion of this Endian.h which is picked up due to case-insensitivity of the file system on macOS (I assume that's still the case)?

All that said, I am inclined to merge this. I will do so in a couple of days.

[palmerc]

All of the current SDKs have these endian files:

% find . -name Endian.h    
./usr/include/Endian.h

% find . -name endian.h
./usr/include/c++/v1/__bit/endian.h
./usr/include/machine/endian.h
./usr/include/arm/endian.h

In cmake you could go with:

// check_symbol_exists(<symbol> <files> <variable>)
check_symbol_exists(be16toh endian.h HAVE_BE16TOH)

[palmerc]

I had failed to update the comment at the bottom of the block to match, but I've done that now.

[botovq]

Thanks. I was hoping we could avoid doing in this super fine grained manner, but yes, it's probably cleaner than what we have now... It's just surprising that I can't find a ready-made solution that covers all our needs. It feels like it should exist...

[palmerc]

What if I just switched the check to this:

if(NOT APPLE AND NOT WIN32)
	check_include_files(endian.h HAVE_ENDIAN_H)
	if(HAVE_ENDIAN_H)
		add_definitions(-DHAVE_ENDIAN_H)
	endif()
endif()

[botovq]

I'm pretty sure that the previous diff was better. This logic introduces an unexpected difference between the CMake and automake builds which will likely bite us down the road.

I'd like to merge the previous version and next time an issue shows up we'll have to sit down and do the grunt work of solving this "the right way."

[palmerc]

Doesn't the other formulation leave windows builds broken? In endian.h, right after the equivalent Apple bit there is similar code defining non-existent functions with the expectation that endian.h does not exist. I can't answer whether or not windows machines have endian.h or not, nor whether they are missing these #defines.

#if defined(_WIN32) && !defined(HAVE_ENDIAN_H)
#include <winsock2.h>

#define be16toh(x) ntohs((x))
#define htobe16(x) htons((x))
#define le32toh(x) (x)
#define be32toh(x) ntohl((x))
#define htole32(x) (x)
#define htobe32(x) ntohl((x))
#define be64toh(x) ntohll((x))

#if !defined(ntohll)
#define ntohll(x)                                                              \
  ((1 == htonl(1))                                                             \
       ? (x)                                                                   \
       : ((uint64_t)ntohl((x)&0xFFFFFFFF) << 32) | ntohl((x) >> 32))
#endif
#if !defined(htonll)
#define htonll(x)                                                              \
  ((1 == ntohl(1))                                                             \
       ? (x)                                                                   \
       : ((uint64_t)htonl((x)&0xFFFFFFFF) << 32) | htonl((x) >> 32))
#endif

#define htobe64(x) ntohll((x))
#define htole64(x) (x)
#define le64toh(x) (x)
#endif /* _WIN32 && !HAVE_ENDIAN_H */

[palmerc]

In any case, the original version works for Apple and that is what matters to me :)

[palmerc]

Did you need anything else for this MR?

[botovq]

No, it's all good. Merged now, thanks.