Skip to content

Conversation

@practicalswift
Copy link
Contributor

@practicalswift practicalswift commented May 21, 2018

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

@fanquake fanquake requested a review from laanwj May 21, 2018 12:21
src/random.cpp Outdated
Copy link
Contributor

@Empact Empact May 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Please re-review :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

src/random.cpp Outdated
Copy link
Contributor

@Empact Empact May 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could maybe benefit from a comment connecting this block with GetOSRand

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Added!

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

Empact commented May 21, 2018

utACK 4ae2178

src/random.cpp Outdated
Copy link
Member

@laanwj laanwj May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@practicalswift practicalswift May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

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

@practicalswift
Copy link
Contributor Author

@fanquake Fixed in #13355 :-)

@practicalswift
Copy link
Contributor Author

@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
Copy link
Member

laanwj commented Jun 7, 2018

@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
Copy link
Member

fanquake commented Jun 8, 2018

@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
Copy link
Contributor Author

@laanwj @fanquake Thanks for your feedback.

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

Please re-review :-)

Copy link

@Nino84 Nino84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mute page

@laanwj
Copy link
Member

laanwj commented Jun 11, 2018

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

utACK a426098

@laanwj laanwj merged commit a426098 into bitcoin:master Jun 11, 2018
laanwj added a commit that referenced this pull request Jun 11, 2018
…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
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 27, 2019
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
@practicalswift practicalswift deleted the openbsd-warnings branch April 10, 2021 19:34
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Jun 28, 2021
…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
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Apr 19, 2022
…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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants