CMessageHeader sanity changes #4940

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2014_09_messageheader_sanity changing 1 files +2 −5

laanwj commented at 8:17 AM on September 18, 2014: member
- Remove spurious pchCommand[1] = 1 in CMessageHeader()
- Make sure that pchCommand is zero-padded if length is shorter than COMMAND_SIZE
- Use strnlen to determine length of pchCommand in GetCommand
I'm afraid that pulltester's old mingw will complain about lack of strnlen. This is not actually a problem with more recent compilers on windows: I verified that it works fine with gitian's version, and it should work with Travis.

CMessageHeader sanity changes

- Remove spurious `pchCommand[1] = 1` in CMessageHeader()
- Make sure that pchCommand is zero-padded if length is shorter than
  COMMAND_SIZE
- Use strnlen to determine length of pcmCommand in GetCommand

62e5f8f961

laanwj added the label P2P on Sep 18, 2014
BitcoinPullTester commented at 8:31 AM on September 18, 2014: none
Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/p4940_62e5f8f96114a872354103ef566b5c74a16154d4/ for binaries and test log.

This could happen for one of several reasons:
1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in qa/pull-tester)
2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
4. The test suite fails on either Linux i386 or Win32
5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)
If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

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.
jgarzik commented at 12:34 PM on September 18, 2014: contributor

ut ACK once pulltester happy
sipa commented at 2:12 AM on September 20, 2014: member

You can simulate strnlen(s, n) with memchr(s, 0, n) in this case.
laanwj commented at 7:59 AM on September 20, 2014: member

Yeah - I'm just a bit reluctant to change the code from the current obvious state just to make a compiler happy that no one is using anymore. As it's working fine in travis and the compiler that is used to build releases. I'd rather just wait for pulltester to die (#4907).
laanwj merged this on Oct 1, 2014
laanwj closed this on Oct 1, 2014
laanwj referenced this in commit 1fbccda8d0 on Oct 1, 2014
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels