nanocurrency / nano-node

Nano is digital currency. Its ticker is: XNO and its currency symbol is: Ӿ
https://nano.org
BSD 3-Clause "New" or "Revised" License
3.47k stars 784 forks source link

Add static program analysis. See cppcheck attached output #2211

Open kvrban opened 4 years ago

kvrban commented 4 years ago

Proposal: Add automatic code with cppcheck. cppcheck found variety issues. Maybe this is useful:

cppcheck -q --enable=performance,warning nano/ [nano/core_test/entry.cpp:16]: (error) Array index -8388608 is out of bounds. [nano/core_test/memory_pool.cpp:13]: (warning) The class 'record_allocations_new_delete_allocator' has 'copy constructor' but lack of 'operator='. [nano/core_test/message_parser.cpp:10]: (warning) Member variable 'test_visitor::node_id_handshake_count' is not initialized in the constructor. [nano/core_test/network.cpp:1046]: (warning) Access of moved variable 'req'. [nano/core_test/network.cpp:1071]: (warning) Access of moved variable 'req'. [nano/core_test/node.cpp:931]: (performance) Function parameter 'text_a' should be passed by reference. [nano/lib/stats.cpp:125]: (performance) Function parameter 'header' should be passed by reference. [nano/lib/stats.cpp:132]: (performance) Function parameter 'type' should be passed by reference. [nano/lib/stats.cpp:132]: (performance) Function parameter 'detail' should be passed by reference. [nano/lib/stats.cpp:132]: (performance) Function parameter 'dir' should be passed by reference. [nano/node/bootstrap.cpp:2348]: (performance) Function parameter 'connection_a' should be passed by reference. [nano/node/ipc.cpp:56]: (performance) Function parameter 'callback_a' should be passed by reference. [nano/node/network.cpp:397]: (performance) Function parameter 'channel_a' should be passed by reference. [nano/node/node.cpp:941]: (performance) Function parameter 'callback_a' should be passed by reference. [nano/node/node.cpp:946]: (performance) Function parameter 'callback_a' should be passed by reference. [nano/node/node.cpp:1314]: (performance) Function parameter 'block_a' should be passed by reference. [nano/node/testing.cpp:185]: (performance) Function parameter 'node_a' should be passed by reference. [nano/node/websocket.cpp:308]: (performance) Function parameter 'topic_a' should be passed by reference.

wezrule commented 4 years ago

I think this looks useful, I had a look at most of the messages and it would help prevent some copies, a use after move and an uninitialised member variable (among other things) which don't seem to be caught by any of other tools. Thanks

kvrban commented 4 years ago

Ok cool. Happy it could help. I recommend to use the tool yourself in the current version 1.88 with the option --enable=all. Because i only used an old version from the debian repository

See current version here: http://cppcheck.sourceforge.net/#news

Further information about static code analysis tools on: https://www.reddit.com/r/cpp/comments/7gaz9j/best_static_code_analysis_tools/

In that thread, they also suggest the commercial tool from Coverity (free for open source projects) https://scan.coverity.com/

wezrule commented 4 years ago

We had discussed using the analyser in clang-tidy before, but more static analysers the better :). I have made many changes related to this in: #2213