Skip to content

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented May 13, 2015

Found while building on Debian 7

NOTE: There is one remaining warning I was not able to figure out:

test/util_tests.cpp:322:5: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

@@ -136,7 +136,7 @@ extern bool fPruneMode;
/** Number of MiB of block files that we're trying to stay below. */
extern uint64_t nPruneTarget;
/** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of chainActive.Tip() will not be pruned. */
static const signed int MIN_BLOCKS_TO_KEEP = 288;
static const unsigned MIN_BLOCKS_TO_KEEP = 288;
Copy link

Choose a reason for hiding this comment

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

Feels strange to not see the type in here, no?

Copy link
Member

Choose a reason for hiding this comment

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

We never use unsigned without type in the code anywhere else, let's just make this unsigned int.

Copy link

Choose a reason for hiding this comment

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

Ping @luke-jr

Copy link
Member Author

Choose a reason for hiding this comment

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

I see 5 other cases in the code, but fine with me.

@@ -496,7 +496,7 @@ void CTxMemPool::removeCoinbaseSpends(const CCoinsViewCache *pcoins, unsigned in
continue;
const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash);
if (fSanityCheck) assert(coins);
if (!coins || (coins->IsCoinBase() && nMemPoolHeight - coins->nHeight < COINBASE_MATURITY)) {
if (!coins || (coins->IsCoinBase() && ((signed long)nMemPoolHeight) - coins->nHeight < COINBASE_MATURITY)) {
Copy link
Member

Choose a reason for hiding this comment

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

coins->nHeight is an int, nMempoolHeight is an unsigned int. Any specific reason for introducing a signed long here?

Copy link
Member Author

Choose a reason for hiding this comment

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

COINBASE_MATURITY is a [signed] int, and we are subtracting.

Copy link
Member

Choose a reason for hiding this comment

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

Right.

@luke-jr luke-jr force-pushed the bugfix_201505_warnings branch from 1ced98a to 130fd3e Compare June 2, 2015 03:48
@jtimon
Copy link
Contributor

jtimon commented Jun 21, 2015

Untested ACK, needs rebase.

Found while building on Debian 7
@luke-jr luke-jr force-pushed the bugfix_201505_warnings branch from 130fd3e to e617fe2 Compare June 23, 2015 08:40
@luke-jr
Copy link
Member Author

luke-jr commented Jun 23, 2015

Rebased

@laanwj laanwj merged commit e617fe2 into bitcoin:master Jul 2, 2015
laanwj added a commit that referenced this pull request Jul 2, 2015
e617fe2 Fix various warnings (Luke Dashjr)
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Jan 10, 2016
Found while building on Debian 7

Github-Pull: bitcoin#6133
Rebased-From: e617fe2
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Jan 10, 2016
Found while building on Debian 7

Github-Pull: bitcoin#6133
Rebased-From: e617fe2
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
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.

4 participants