Skip to content

Conversation

jonasschnelli
Copy link
Contributor

Fixes an integer comparison of different signs (which errors out on -Werror,-Wsign-compare). Introduced in #21121.

See https://bitcoinbuilds.org/index.php?ansilog=982c61cf-6969-4001-bebc-dc215e5d29a4.log

@maflcko
Copy link
Member

maflcko commented Feb 18, 2021

review ACK bedb8d8

unrelated: It would be good to have steps to reproduce for this on Linux (or at least in our ci configs). This is popping up every other week: #19123 , #21159, #19493, #18637 (comment), ...

@maflcko maflcko changed the title Avoid comparision of integers with different signs test: Avoid comparision of integers with different signs Feb 18, 2021
Copy link
Contributor

@amitiuttarwar amitiuttarwar left a comment

Choose a reason for hiding this comment

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

ACK bedb8d8

do we know the difference between bitcoin builds & our github CI that leads to the warning there but not here?

@jonasschnelli
Copy link
Contributor Author

Cirrus mac build has -werror for sign-compare enabled (https://cirrus-ci.com/task/4665160058011648?command=ci#L1296). Could it be due different implementation of boosts equal_impl()?

@maflcko
Copy link
Member

maflcko commented Feb 19, 2021

The cross build should be using depends, which is pinned to a boost and compiler version

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK bedb8d8

@fanquake fanquake merged commit f093310 into bitcoin:master Feb 19, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 19, 2021
…nt signs

bedb8d8 Avoid comparision of integers with different signs (Jonas Schnelli)

Pull request description:

  Fixes an integer comparison of different signs (which errors out on `-Werror,-Wsign-compare`). Introduced in bitcoin#21121.

  See https://bitcoinbuilds.org/index.php?ansilog=982c61cf-6969-4001-bebc-dc215e5d29a4.log

ACKs for top commit:
  MarcoFalke:
    review ACK bedb8d8
  amitiuttarwar:
    ACK bedb8d8
  vasild:
    ACK bedb8d8

Tree-SHA512: cb22a6239a1fc9d0be5573bf6ae4ec379eb7398c88edc8fa2ae4fd721f37f9ca3724896c1ac16de14a5286888a0b631813da32cb62d177ffbf9b2c31e716a7aa
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants