Clang cleanup #2123

pull apoelstra wants to merge 2 commits into bitcoin:master from apoelstra:clang-cleanup changing 2 files +0 −6

apoelstra commented at 6:19 AM on December 22, 2012: contributor

Yesterday LLVM 3.2 was released, as well as a corresponding version of the clang compiler and static analyzer.

I ran the static analyzer over the bitcoin code and picked up a couple of assignments to variables that are never read. These two patches remove those assigments.
BitcoinPullTester commented at 6:33 AM on December 22, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/75fafc30d24648588801baa50242c6d05cd5d75b for binaries and test log.
serialize.h: Remove assignment to variable that is never read a8b022429a
irc.cpp: remove unused variable assignments 3d12bdaa7c

in src/serialize.h:None in 75fafc30d2 outdated

1044 | @@ -1045,10 +1045,7 @@ class CDataStream
1045 |          if (nReadPosNext >= vch.size())
1046 |          {
1047 |              if (nReadPosNext > vch.size())
1048 | -            {
1049 | -                setstate(std::ios::failbit, "CDataStream::ignore() : end of data");
1050 | -                nSize = vch.size() - nReadPos;
1051 | -            }
1052 | +              setstate(std::ios::failbit, "CDataStream::ignore() : end of data");

Diapolo commented at 1:19 PM on December 22, 2012:

This needs 4 spaced as indention :).

BitcoinPullTester commented at 5:19 PM on December 22, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/3d12bdaa7ce61bc6dbe395a7f083325f041b4a00 for binaries and test log.
Diapolo commented at 4:08 PM on December 26, 2012: none

I'm interested in how that analyzer outputs found stuff, can you perhaps post that? As I'm on Windows it's not that easy to get such stuff working over here ;).
apoelstra commented at 2:17 AM on December 27, 2012: contributor

On Linux, you can get the analyzer to run by adding

CXX=clang++ --analyze

right above the "LINK=$(CXX)" line in makefile.unix, then do a full build. It will output its warnings as though it were a compiler (though it won't actually compile anything). I'm not familiar with the Windows build process, sorry.
laanwj commented at 3:55 PM on January 5, 2013: member

I've also seen these warnings by the analyzer, but I'm not so sure about removing the code. It's there to make maintenance easier (ie, will make it somewhat more robust when adding code by making sure hSOCKET will never have an invalid value, and nSize will always be up to date). Compilers are generally smart enough to remove dead code like this in the binary.
sipa commented at 1:12 AM on January 6, 2013: member

agree with @laanwj

Being able to get useful information through a static analyzer is nice, though.
Diapolo commented at 2:58 AM on January 6, 2013: none

It would be indeed better to keep these 2 and so I'm fine with just closing this.
gavinandresen commented at 7:54 PM on January 14, 2013: contributor

Closing this, I like the belt-and-suspenders easier-to-maintain code.
gavinandresen closed this on Jan 14, 2013
DrahtBot locked this on Sep 8, 2021

Contributors