Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 #13294

practicalswift commented at 12:05 PM on May 21, 2018: contributor

Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 (OpenBSD clang version 5.0.1, based on LLVM 5.0.1):

random.cpp:182:13: warning: unused function 'GetDevURandom' [-Wunused-function]
static void GetDevURandom(unsigned char *ent32)
            ^

txmempool.cpp:707:45: warning: comparison of integers of different signs: 'uint64_t' (aka 'unsigned long long') and 'long long' [-Wsign-compare]
        assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

practicalswift force-pushed on May 21, 2018

fanquake requested review from laanwj on May 21, 2018

in src/random.cpp:178 in fb5ed74c99 outdated

 174 | @@ -175,7 +175,7 @@ static void RandAddSeedPerfmon()
 175 |  #endif
 176 |  }
 177 |  
 178 | -#ifndef WIN32
 179 | +#if !defined(WIN32) && !(defined(HAVE_GETENTROPY) && defined(__OpenBSD__))

Empact commented at 6:06 PM on May 21, 2018:

I think we need to encode the full GetOSRand elif tree up to that point. i.e.: #if !defined(WIN32) && (defined(HAVE_SYS_GETRANDOM) || !(defined(HAVE_GETENTROPY) && defined(__OpenBSD__)))

practicalswift commented at 8:08 PM on May 21, 2018:

Updated. Please re-review :-)

practicalswift force-pushed on May 21, 2018

in src/txmempool.cpp:707 in f86c2d9408 outdated

 703 | @@ -704,7 +704,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
 704 |          assert(setChildrenCheck == GetMemPoolChildren(it));
 705 |          // Also check to make sure size is greater than sum with immediate children.
 706 |          // just a sanity check, not definitive that this calc is correct...
 707 | -        assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
 708 | +        assert(it->GetSizeWithDescendants() >= (uint64_t)(childSizes + it->GetTxSize()));

MarcoFalke commented at 8:17 PM on May 21, 2018:

Might as well change childSizes into uint64_t child_sizes = 0; a few lines up.

practicalswift commented at 8:20 PM on May 21, 2018:

Good point! Updated. Please re-review! :-)

in src/random.cpp:180 in f86c2d9408 outdated

 174 | @@ -175,7 +175,18 @@ static void RandAddSeedPerfmon()
 175 |  #endif
 176 |  }
 177 |  
 178 | -#ifndef WIN32
 179 | +#if defined(WIN32)

Empact commented at 8:19 PM on May 21, 2018:

Could maybe benefit from a comment connecting this block with GetOSRand

practicalswift commented at 8:31 PM on May 21, 2018:

Good point. Added!

practicalswift force-pushed on May 21, 2018

Empact commented at 9:22 PM on May 21, 2018: member

utACK 4ae2178

laanwj added the label Linux/Unix on May 23, 2018

in src/random.cpp:180 in 4ae2178f20 outdated

 174 | @@ -175,7 +175,20 @@ static void RandAddSeedPerfmon()
 175 |  #endif
 176 |  }
 177 |  
 178 | -#ifndef WIN32
 179 | +// URANDOM_FALLBACK should be defined if GetDevURandom(...) is used as a
 180 | +// fallback in GetOSRand(...).
 181 | +#if defined(WIN32)

laanwj commented at 3:22 PM on May 29, 2018:

This is correct but I... think this is somewhat overdoing it, creating an ifdef forest just to avoid a warning.

practicalswift commented at 9:56 PM on May 29, 2018:

This code just replicates the already existing #ifdef forest (implicit in the code below) but if we can avoid the warning in a smarter way I'm all ears. What method do you suggest using for avoiding the warning? :-)

Empact commented at 10:54 PM on May 29, 2018:

One option would be to forward-declare the method here and define it below based on a define that occurs inside the relevant #ifs.

fanquake commented at 6:27 AM on May 31, 2018: member

@practicalswift Did you have any issues with gmake check? i.e #13337.

practicalswift commented at 6:01 AM on June 6, 2018: contributor

@fanquake Fixed in #13355 :-)

practicalswift commented at 5:18 PM on June 7, 2018: contributor

@laanwj Would you be okay with the other non-#ifdef change in this file? If so I can split that one out. Getting rid of the OpenBSD warnings would be nice :-)

laanwj commented at 11:18 PM on June 7, 2018: member

@laanwj Would you be okay with the other non-#ifdef change in this file? If so I can split that one out. Getting rid of the OpenBSD warnings would be nice :-)

Yes, my problem here is the duplicated #ifdef forest, the rest is fine...

fanquake commented at 1:01 AM on June 8, 2018: member

@practicalswift I don't think there's a need to split anything out of here. Probably easier to just drop the random.cpp changes from this PR. They can be revisited later with a different approach.

Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 a426098572

practicalswift force-pushed on Jun 10, 2018

practicalswift commented at 9:02 AM on June 10, 2018: contributor

@laanwj @fanquake Thanks for your feedback.

I've now dropped the changes to random.cpp from this PR.

Please re-review :-)

Nino84 commented at 9:18 AM on June 10, 2018: none

Mute page

laanwj commented at 1:06 PM on June 11, 2018: member

@Nino84 That doesn't work and only pollutes the topic, you need to use github's "unsubscribe" button.

utACK a426098572884349a3d9081187eaeb999f6e2c5a

laanwj merged this on Jun 11, 2018

laanwj closed this on Jun 11, 2018

laanwj referenced this in commit 26c93edf1d on Jun 11, 2018

jasonbcox referenced this in commit b5e2c50e15 on Sep 27, 2019

practicalswift deleted the branch on Apr 10, 2021

UdjinM6 referenced this in commit 5744b6d573 on Jun 20, 2021

UdjinM6 referenced this in commit 96f6991ed4 on Jun 24, 2021

UdjinM6 referenced this in commit cefa202f43 on Jun 26, 2021

UdjinM6 referenced this in commit c3a2c54a7c on Jun 26, 2021

UdjinM6 referenced this in commit ab36b43daf on Jun 26, 2021

PastaPastaPasta referenced this in commit fed64687ee on Jun 27, 2021

PastaPastaPasta referenced this in commit 234d66a706 on Jun 28, 2021

PastaPastaPasta referenced this in commit d33131ff28 on Jun 28, 2021

UdjinM6 referenced this in commit e47f2e9172 on Jun 28, 2021

PastaPastaPasta referenced this in commit 3ce9e779d6 on Jun 29, 2021

gades referenced this in commit b1bc40ec69 on Apr 19, 2022

DrahtBot locked this on Aug 18, 2022