Prefer using std::numeric_limits to C #6062

sinetek commented at 12:55 PM on April 25, 2015: contributor

This fixes compilation under DragonFlyBSD.

theuni commented at 1:12 PM on April 25, 2015: member

This is not our code, so we're best off not modifying it is possible. Best options imo are either:

define __STDC_LIMIT_MACROS for the leveldb build, which should get SIZE_MAX defined
push this change upstream and catch it on the next sync

sinetek commented at 1:24 PM on April 25, 2015: contributor

The inclusion of leveldb made me think it has been modified. I agree it would be better to modify the Makefile, something like -D__STDC_LIMIT_MACROS. ?

sinetek commented at 1:25 PM on April 25, 2015: contributor

That brings us to the same position because the Makefile is part of leveldb.

theuni commented at 1:26 PM on April 25, 2015: member

For the hard-coded approach:

diff --git a/src/Makefile.am b/src/Makefile.am
index 37184b6..aed47db 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -14,7 +14,7 @@ $(LIBLEVELDB): $(LIBMEMENV)
 $(LIBLEVELDB) $(LIBMEMENV): [@echo](/bitcoin-bitcoin/contributor/echo/) "Building LevelDB ..." && $(MAKE) -C $(@D) $(@F) CXX="$(CXX)" \
          CC="$(CC)" PLATFORM=$(TARGET_OS) AR="$(AR)" $(LEVELDB_TARGET_FLAGS) \
-          OPT="$(CXXFLAGS) $(CPPFLAGS)"
+          OPT="$(CXXFLAGS) $(CPPFLAGS) -D__STDC_LIMIT_MACROS"
 endif

 BITCOIN_CONFIG_INCLUDES=-I$(builddir)/config

Fix for correctly including climits on certain platforms {DragonFlyBSD}. ff7fe8b19e

sinetek commented at 3:09 PM on April 25, 2015: contributor

Works.

laanwj commented at 12:44 PM on April 26, 2015: member

You should still aim to get this merged upstream; apparently it doesn't support DragonFlyBSD at the moment.

laanwj added the label Build system on Apr 29, 2015

laanwj commented at 7:08 AM on April 29, 2015: member

utACK in any case - as a workarond until the issue is resolved upstream

sinetek commented at 10:41 AM on April 29, 2015: contributor

I'll see if I can push upstream too.

sinetek commented at 10:53 AM on April 29, 2015: contributor

There must be a difference in the build systems, as upstream compiles fine.

g++ -I. -I./include -std=c++0x -fno-builtin-memcmp -D_REENTRANT -DOS_DRAGONFLYBSD -DLEVELDB_PLATFORM_POSIX -DLEVELDB_ATOMIC_PRESENT -O2 -DNDEBUG -c helpers/memenv/memenv.cc -o helpers/memenv/memenv.o

laanwj commented at 12:45 PM on April 30, 2015: member

Ok that's strange, I wonder why it would be needed here then. Maybe because we override the CFLAGS?

laanwj commented at 3:22 PM on May 8, 2015: member

@theuni any idea what could be the issue here? according to @sinetek the upstream leveldb compiles fine, do our overrides create this issue then?

theuni commented at 5:17 PM on May 8, 2015: member

I'm not sure what could be causing the difference. We could defined the freebsd/openbsd/dflybsd targets in configure and pass them on to leveldb (which we should do, to fix cross builds), but they should be correctly detected for native builds already. @sinetek could you please paste the failing compile-line so that we can see the difference? My only guesses are that we somehow define a different target, or our flags make the -std=c++0x change fail so that we build in c++98 mode.

laanwj commented at 8:48 AM on May 16, 2015: member

Going to merge this. Normally a build fix for a platform should only affect that platform, but I'm reasonably confident that it won't have adverse effects on other platforms.

laanwj merged this on May 16, 2015

laanwj closed this on May 16, 2015

laanwj referenced this in commit ec82d8c99c on May 16, 2015

MarcoFalke locked this on Sep 8, 2021