Default to defining endian-conversion DECLs in compat w/o config

[TheBlueMatt]

While this isn't a supported build configuration, some build
systems need to build without going through our autotools steps,
so defaulting to something sane may make it easier to build.

Specifically, this fixes the inability to build
rust-bitcoinconsensus on some non-x86 platforms. It needs to build
without our autotools/configure steps to ensure correct compile
args are passed from the rust build system to gcc. Converting the
args from the rust build system to gcc would be a lot of
unmaintainable work.

[laanwj]

I'm not sure making this assumption makes sense - the environment might or might not have those functions, without configure (or similar) that's impossible to know.

You could also define the -DHAVE_DECL_BE16TOH etc on the compiler command line, or ship with a pre-existing config.h?

[TheBlueMatt]

In my use-case (rust-bitcoinconsensus) we don't know anything more about the environment than non-configure Bitcoin. Re-implementing just enough of Bitcoin Core's configure to define the appropriate symbols seems like the wrong way to go (and would be entirely unmaintainable), actually running autogen/configure would also be a pain as we'd have to figure out what environment we're targeting and appropriately convert gcc flags to configure flags (as well as probably reimplement parts of the "compile a C dependency" rust module that is well-maintained by others). Not sure of another easy solution aside from this, but if we don't take it we should almost certainly add a #ifndef HAVE_CONFIG_H #error "You need to run configure first" in a bunch of places.

[theuni]

Agree with @laanwj.

Also agree with @TheBlueMatt about this being annoying, though. I've run into this exact issue as well, usually when I try to copy/paste sha256.cpp into a quick project.

As these are non-standard, I don't think there's a default that makes more sense than any other. For example, I believe that these aren't present on macOS. And I don't think we can assume that they're always defines when they do exist.

Imo what makes the most sense is (using htobe16 as an example):

• rename all external calls to htobe16_int. I don't think there should be (m)any of these outside of crypto/.
• If HAVE_CONFIG_H, htobe16_int is defined as htobe16, essentially as it's done now
• If no HAVE_CONFIG_H and HAVE_DECL_HTOBE16, same as above, so that it can be overridden by other projects that know about their environment
• If no HAVE_CONFIG_H and no HAVE_DECL_HTOBE16, htobe16_int is an explicit manual conversion function

[TheBlueMatt]

I mean I'm open to doing the big refactor and proxying everything through inline "int" functions, but I figured that would be a bigger deal than just skipping compile of the conversion functions in the case where they're already defined (ie its super obvious we dont need them). Its not supposed to be a foolproof solution, just a simple fix that happens to be sufficient in some cases, and creates ever-so-slightly more robust defaults.

[theuni]

I was a bit too hasty in my review of this.

I see where you're coming from now. This won't fix all non-autotools builds, but it should be strictly an improvement in cases where these defines exist.

I'd still prefer _int functions since we only use these in crypto/ and serialize.h, but barring that, I see now that it's hard to argue against this.

utACK 150b2f0

[laanwj]

utACK - with the #ifdef htole16 condition it makes some sense, there's no implicit assumption now.
I suppose there's no way to shortcut this like

#define HAVE_DECL_HTOBE16 defined(htobe16)

[fanquake]

I can include this in #12967.

[maflcko]

@TheBlueMatt Is this for backport?

[TheBlueMatt]

I'd prefer it, as indicated at https://botbot.me/freenode/bitcoin-core-dev/2018-04-24/?msg=99324321&page=2

[maflcko]

@TheBlueMatt It is included in #12967 (you are very welcome to review it)