CBigNum::setint64() does 'n <<= 8', where n is of type "long long".
This leads to shifting onto and past the sign bit, which is undefined
behavior in C++11 and can cause problems in the future.
Acquire an exclusive, advisory lock before sending output to debug.log
and release it when we're done. This should avoid output from multiple
threads being interspersed in the log file.
We can't use CRITICAL_SECTION machinery for this because the debug log
is written during startup and shutdown when that machinery is not
available.
(Thanks to Gavin for pointing out the CRITICAL_SECTION problems based
on his earlier work in this area)
Any problems seen during deserialization will throw an uncaught
exception, crashing the entire bitcoin process. Properly return an
error instead, so that we may at least log the error and gracefully
shutdown other portions of the app.
Immediately issue a "getblocks", instead of a "getdata" (which will
trigger the relevant "inv" to be sent anyway), and only do so when
the previous set of invs led us into a known and attached part of
the block tree.
Pull request #948 introduced a fix for nodes stuck on a long side branch
of the main chain. The fix was non-functional however, as the additional
getdata request was created in a first step of processing, but dropped
in a second step as it was considered redundant. This commits fixes it
by sending the request directly.
In cases of very large reorganisations (hundreds of blocks), a situation
may appear where an 'inv' is sent as response to a 'getblocks', but the
last block mentioned in the inv is already known to the receiver node.
However, the supplying node uses a request for this last block as a
trigger to send the rest of the inv blocks. If it never comes, the block
chain download is stuck.
This commit makes the receiver node always request the last inv'ed block,
even if it is already known, to prevent this problem.
Rather than storing ftell(3)'s return value -- a long -- in an
unsigned int, we store and check a properly typed temp. Then, assured a
non-negative value, we store in nBlockPosRet.
In ISO C++, the signedness of 'char' is undefined. On some platforms (e.g.
ARM), 'char' is an unsigned type, but some of the code relies on 'char' being
signed (as it is on x86). This is indicated by compiler warnings like this:
bignum.h: In constructor 'CBigNum::CBigNum(char)':
bignum.h:81:59: warning: comparison is always true due to limited range of data type [-Wtype-limits]
util.cpp: In function 'bool IsHex(const string&)':
util.cpp:427:28: warning: comparison is always false due to limited range of data type [-Wtype-limits]
In particular, IsHex erroneously returned true regardless of the input
characters, as long as the length of the string was a positive multiple of 2.
Note: For testing, it's possible using GCC to force char to be unsigned by
adding the -funsigned-char parameter to xCXXFLAGS.
Conflict:
* cs_main in ProcessMessages() (before calling ProcessMessages)
* cs_vSend in CNode::BeginMessage
versus:
* cs_vSend in ThreadMessageHandler2 (before calling SendMessages)
* cs_main in SendMessages
Even though cs_vSend is a try_lock, if it succeeds simultaneously with
the locking of cs_main in ProcessMessages(), it could cause a deadlock.
