Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 by practicalswift · Pull Request #13294 · bitcoin/bitcoin

practicalswift · 2018-05-21T12:05:44Z

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());
               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Empact · 2018-05-21T18:06:44Z

src/random.cpp

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__)))

Updated. Please re-review :-)

maflcko · 2018-05-21T20:17:27Z

src/txmempool.cpp

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

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

Empact · 2018-05-21T20:19:05Z

src/random.cpp

Could maybe benefit from a comment connecting this block with GetOSRand

Good point. Added!

Empact · 2018-05-21T21:22:28Z

utACK 4ae2178

laanwj · 2018-05-29T15:22:15Z

src/random.cpp

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

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? :-)

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 · 2018-05-31T06:27:05Z

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

practicalswift · 2018-06-06T06:01:38Z

@fanquake Fixed in #13355 :-)

practicalswift · 2018-06-07T17:18:36Z

@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 · 2018-06-07T23:18:03Z

@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 · 2018-06-08T01:01:02Z

@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.

practicalswift · 2018-06-10T09:02:02Z

@laanwj @fanquake Thanks for your feedback.

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

Please re-review :-)

Nino84

Mute page

laanwj · 2018-06-11T13:06:26Z

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

utACK a426098

…k OpenBSD 6.3 a426098 Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 (practicalswift) Pull request description: 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()); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` Tree-SHA512: da2ae86218054b10659ea694179433700ac91de8022e06007348168ed5adc3d8c4ad3b32a3fc5783a2cdf1ca7425aff586b839200dd3b226ebff72a7df15f120

Summary: Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 (OpenBSD clang version 5.0.1, based on LLVM 5.0.1): Backport of Bitcoin Core PR13294 bitcoin/bitcoin#13294 Test Plan: `make check-all` is OK on my platform (Linux 5.2.15-200.fc30.x86_64 x86_64) I don't have an OpenBSD 6.3 to test it. It doesn't seem that the changes are going to have problems on it. Reviewers: Fabien, #bitcoin_abc, deadalnix, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D4141

…er stock OpenBSD 6.3 a426098 Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 (practicalswift) Pull request description: 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()); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` Tree-SHA512: da2ae86218054b10659ea694179433700ac91de8022e06007348168ed5adc3d8c4ad3b32a3fc5783a2cdf1ca7425aff586b839200dd3b226ebff72a7df15f120

practicalswift force-pushed the openbsd-warnings branch from 44d565f to fb5ed74 Compare May 21, 2018 12:07

fanquake requested a review from laanwj May 21, 2018 12:21

Empact reviewed May 21, 2018

View reviewed changes

practicalswift force-pushed the openbsd-warnings branch from fb5ed74 to f86c2d9 Compare May 21, 2018 20:07

maflcko reviewed May 21, 2018

View reviewed changes

Empact reviewed May 21, 2018

View reviewed changes

practicalswift force-pushed the openbsd-warnings branch 2 times, most recently from 71f88d1 to 4ae2178 Compare May 21, 2018 20:31

laanwj added the Linux/Unix label May 23, 2018

laanwj reviewed May 29, 2018

View reviewed changes

Fix compiler warnings emitted when compiling under stock OpenBSD 6.3

a426098

practicalswift force-pushed the openbsd-warnings branch from 4ae2178 to a426098 Compare June 10, 2018 09:01

Nino84 reviewed Jun 10, 2018

View reviewed changes

laanwj merged commit a426098 into bitcoin:master Jun 11, 2018

Bushstar mentioned this pull request Jun 12, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#343

Merged

laanwj mentioned this pull request Nov 22, 2019

lib: fix a compiler warning: unused GetDevURandom() #17563

Merged

practicalswift deleted the openbsd-warnings branch April 10, 2021 19:34

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Conversation

practicalswift commented May 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Empact May 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift May 21, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko May 21, 2018

Choose a reason for hiding this comment

Uh oh!

practicalswift May 21, 2018

Choose a reason for hiding this comment

Uh oh!

Empact May 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift May 21, 2018

Choose a reason for hiding this comment

Uh oh!

Empact commented May 21, 2018

Uh oh!

laanwj May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Empact May 29, 2018

Choose a reason for hiding this comment

Uh oh!

fanquake commented May 31, 2018

Uh oh!

practicalswift commented Jun 6, 2018

Uh oh!

practicalswift commented Jun 7, 2018

Uh oh!

laanwj commented Jun 7, 2018

Uh oh!

fanquake commented Jun 8, 2018

Uh oh!

practicalswift commented Jun 10, 2018

Uh oh!

Nino84 left a comment

Choose a reason for hiding this comment

Uh oh!

laanwj commented Jun 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

practicalswift commented May 21, 2018 •

edited

Loading

Empact May 21, 2018 •

edited

Loading

Empact May 21, 2018 •

edited

Loading

laanwj May 29, 2018 •

edited

Loading

practicalswift May 29, 2018 •

edited

Loading