Fixing warning C4146: unary minus operator applied to unsigned type, result still unsigned #4823

ENikS commented at 9:40 PM on September 2, 2014: contributor

Making compiler happy

Fixing C4146 warning be2e45ef59

sipa commented at 9:47 PM on September 2, 2014: member

Untested ACK, but please don't change the indentation.

TheBlueMatt commented at 10:36 PM on September 2, 2014: member

ut ACK given indentation fix.

Indentation fix ea1e4fe3ac

BitcoinPullTester commented at 10:57 PM on September 2, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4823_ea1e4fe3ac1aa5a5421e864d67b3201b915be041/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

in src/util.cpp:None in ea1e4fe3ac

 616 | @@ -617,7 +617,7 @@ void ShrinkDebugFile()
 617 |      {
 618 |          // Restart the file with some of the end
 619 |          std::vector <char> vch(200000,0);
 620 | -        fseek(file, -vch.size(), SEEK_END);
 621 | +        fseek(file, -((long)vch.size()), SEEK_END);

laanwj commented at 10:11 AM on September 4, 2014:

Shouldn't this be ssize_t instead of long?

ENikS commented at 3:14 PM on September 5, 2014:

fseek defined with long. I don't see much reason to invent something new there

laanwj commented at 4:43 PM on September 5, 2014:

My aversion against long is that it is 64-bit on win64 and 32-bit on 64-bit linux and osx. But indeed, as fseek takes a long it makes sense here.

in src/script.h:None in ea1e4fe3ac

 164 | @@ -165,7 +165,7 @@ class CScriptNum
 165 |        // If the input vector's most significant byte is 0x80, remove it from
 166 |        // the result's msb and return a negative.
 167 |        if (vch.back() & 0x80)
 168 | -          return -(result & ~(0x80ULL << (8 * (vch.size() - 1))));
 169 | +          return -((int64_t)(result & ~(0x80ULL << (8 * (vch.size() - 1)))));

jgarzik commented at 12:28 PM on September 4, 2014:

This needs testing to ensure we are returning the proper result. What input data will achieve this? cc @theuni

sipa commented at 2:57 AM on September 16, 2014:

Will achieve what?

laanwj commented at 7:05 AM on September 19, 2014:

We can enumerate the possible values of vch.size() 1..nMaxNumSize and see if it can produce a cast with undefined behavior (where the sign bit is set)

vch.size() | max result       | mask             | max casted value 
-----------|------------------|------------------|------------------
 1         | 00000000000000ff | ffffffffffffff7f | 000000000000007f
 2         | 000000000000ffff | ffffffffffff7fff | 0000000000007fff
 3         | 0000000000ffffff | ffffffffff7fffff | 00000000007fffff
 4         | 00000000ffffffff | ffffffff7fffffff | 000000007fffffff

max result is the maximum value of result for that vector length
mask is ~(0x80ULL << (8 * (vch.size() - 1)))
max casted value is the maximum result ANDed with the mask. This is the maximum value that will ever be fed to the (int64_t) cast.

The value that goes into the (int64_t) cast never has bit 63 set, hence there is no problem.

sipa commented at 4:36 PM on September 19, 2014:

utACK

laanwj referenced this in commit f66f986a25 on Sep 25, 2014

laanwj commented at 1:27 PM on September 25, 2014: member

Merged via f66f986

laanwj closed this on Sep 25, 2014

ENikS deleted the branch on Sep 25, 2014

MarcoFalke locked this on Sep 8, 2021