CppCheck cleanup (#975)

* Add support for inline cppcheck suppressions
* Clean out all known cppcheck issues
* Terminate toll-gate on found cppcheck issues
This commit is contained in:
Patrick Fallberg
2017-11-07 20:37:36 +01:00
committed by Thomas Mørch
parent e2ac85afbb
commit 2d5404de97
23 changed files with 86 additions and 50 deletions

View File

@@ -5,7 +5,7 @@ def cppCheck(config) {
sh """#!/bin/bash +x
cd ${config.repository_root}
echo "Doing cppcheck for AVR..."
find . -type f \\( -iname \\*.c -o -iname \\*.cpp -o -iname \\*.ino \\) | cppcheck -j 4 --file-list=- --enable=style,information --platform=.mystools/cppcheck/config/avr.xml --suppressions-list=.mystools/cppcheck/config/suppressions.cfg --includes-file=.mystools/cppcheck/config/includes.cfg --language=c++ --xml --xml-version=2 2> cppcheck-avr.xml
find . -type f \\( -iname \\*.c -o -iname \\*.cpp -o -iname \\*.ino \\) | cppcheck -j 4 --force --file-list=- --enable=style,information --platform=.mystools/cppcheck/config/avr.xml --suppressions-list=.mystools/cppcheck/config/suppressions.cfg --includes-file=.mystools/cppcheck/config/includes.cfg --language=c++ --inline-suppr --xml --xml-version=2 2> cppcheck-avr.xml
cppcheck-htmlreport --file="cppcheck-avr.xml" --title="cppcheck-avr" --report-dir=cppcheck-avr_cppcheck_reports --source-dir=."""
publishHTML([allowMissing: false, alwaysLinkToLastBuild: false, keepAll: true,
@@ -29,9 +29,8 @@ def cppCheck(config) {
"grep -q \"<td>0</td><td>total</td>\" cppcheck-avr_cppcheck_reports/index.html || exit_code=\$?\n"+
"exit \$((exit_code == 0 ? 0 : 1))")
if (ret == 1) {
config.pr.setBuildStatus(config, 'SUCCESS', 'Toll gate (Code analysis - Cppcheck)', 'Issues found (but are not considered critical)', '${BUILD_URL}CppCheck_AVR/index.html')
//currentBuild.result = 'UNSTABLE'
//error 'Terminating due to Cppcheck error'
config.pr.setBuildStatus(config, 'ERROR', 'Toll gate (Code analysis - Cppcheck)', 'Issues found', '${BUILD_URL}CppCheck_AVR/index.html')
error 'Terminating due to Cppcheck error'
} else {
config.pr.setBuildStatus(config, 'SUCCESS', 'Toll gate (Code analysis - Cppcheck)', 'Pass', '')
}

View File

@@ -1,8 +1,3 @@
-IC:\Program Files (x86)\Arduino\hardware\arduino\avr\cores\arduino
-IC:\Program Files (x86)\Arduino\hardware\tools\avr\avr\include
-Idrivers\Linux
-Idrivers\ATSHA204
-Icore
-DMY_SIGNING_ATSHA204
-DMY_SIGNING_SOFT
-DARDUINO_ARCH_AVR

View File

@@ -1,5 +1,4 @@
toomanyconfigs
missingInclude
missingIncludeSystem
ConfigurationNotChecked
unmatchedSuppression
// This suppression is because the problem is in an in-lined macro so in-line-suppressions does not appear to take effect
unreadVariable:*/MyHwNRF5.cpp

View File

@@ -9,6 +9,7 @@ OPTIONS="--quiet \
--library=${LIBRARY:-avr} \
--platform="${TOOLCONFIG}"/${PLATFORM:-avr.xml} \
--includes-file="${TOOLCONFIG}"/includes.cfg \
--inline-suppr \
--suppressions-list="${TOOLCONFIG}"/suppressions.cfg"
echo $OPTIONS

View File

@@ -49,7 +49,11 @@ MyMessage _ethernetMsg;
#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
typedef struct {
// Suppress the warning about unused members in this struct because it is used through a complex
// set of preprocessor directives
// cppcheck-suppress unusedStructMember
char string[MY_GATEWAY_MAX_RECEIVE_LENGTH];
// cppcheck-suppress unusedStructMember
uint8_t idx;
} inputBuffer;

View File

@@ -58,7 +58,9 @@ void ledsProcess()
}
prevTime = hwMillis();
#if defined(MY_DEFAULT_RX_LED_PIN) || defined(MY_DEFAULT_TX_LED_PIN) || defined(MY_DEFAULT_ERR_LED_PIN)
uint8_t state;
#endif
// For an On/Off ratio of 4, the pattern repeated will be [on, on, on, off]
// until the counter becomes 0.

View File

@@ -128,7 +128,6 @@ bool firmwareOTAUpdateProcess(void)
MY_SERIALDEVICE.print(prbuf);
}
OTA_DEBUG(PSTR("\n"));
8
}
#endif
_firmwareBlock--;

View File

@@ -59,7 +59,6 @@ bool protocolParse(MyMessage &message, char *inputString)
break;
case 5: // Variable value
if (command == C_STREAM) {
blen = 0;
while (*str) {
uint8_t val;
val = protocolH2i(*str++) << 4;
@@ -125,8 +124,6 @@ bool protocolMQTTParse(MyMessage &message, char* topic, uint8_t* payload, unsign
{
char *str, *p;
uint8_t i = 0;
uint8_t bvalue[MAX_PAYLOAD];
uint8_t blen = 0;
uint8_t command = 0;
if (topic != strstr(topic, MY_MQTT_SUBSCRIBE_TOPIC_PREFIX)) {
// Prefix doesn't match incoming topic
@@ -175,9 +172,10 @@ bool protocolMQTTParse(MyMessage &message, char* topic, uint8_t* payload, unsign
// Add payload
if (command == C_STREAM) {
blen = 0;
uint8_t val;
uint8_t bvalue[MAX_PAYLOAD];
uint8_t blen = 0;
while (*payload) {
uint8_t val;
val = protocolH2i(*payload++) << 4;
val += protocolH2i(*payload++);
bvalue[blen] = val;

View File

@@ -111,6 +111,8 @@ void signerInit(void)
#endif
#if (defined (MY_ENCRYPTION_FEATURE) || defined (MY_SIGNING_FEATURE)) &&\
!defined (MY_SIGNING_SIMPLE_PASSWD)
// Suppress this warning since it is only fixed on Linux builds and this keeps the code more tidy
// cppcheck-suppress knownConditionTrueFalse
if (!signerInternalValidatePersonalization()) {
SIGN_DEBUG(PSTR("!SGN:PER:TAMPERED\n"));
#if defined(MY_SIGNING_FEATURE)

View File

@@ -103,10 +103,8 @@ bool signerAtsha204SoftInit(void)
_signing_node_serial_info[8] = getNodeId();
}
#else
if (init_ok) {
hwReadConfigBlock((void*)_signing_hmac_key, (void*)EEPROM_SIGNING_SOFT_HMAC_KEY_ADDRESS, 32);
hwReadConfigBlock((void*)_signing_node_serial_info, (void*)EEPROM_SIGNING_SOFT_SERIAL_ADDRESS, 9);
}
hwReadConfigBlock((void*)_signing_hmac_key, (void*)EEPROM_SIGNING_SOFT_HMAC_KEY_ADDRESS, 32);
hwReadConfigBlock((void*)_signing_node_serial_info, (void*)EEPROM_SIGNING_SOFT_SERIAL_ADDRESS, 9);
#endif
if (!memcmp(_signing_node_serial_info, reset_serial, 9)) {
unique_id_t uniqueID;

View File

@@ -940,6 +940,8 @@ void transportProcessMessage(void)
void transportInvokeSanityCheck(void)
{
// Suppress this because the function may return a variable value in some configurations
// cppcheck-suppress knownConditionTrueFalse
if (!transportSanityCheck()) {
TRANSPORT_DEBUG(PSTR("!TSF:SAN:FAIL\n")); // sanity check fail
transportSwitchSM(stFailure);

View File

@@ -407,11 +407,11 @@ byte AES::cbc_encrypt (byte * plain, byte * cipher, int n_block)
/******************************************************************************/
byte AES::decrypt (byte plain [N_BLOCK], byte cipher [N_BLOCK])
byte AES::decrypt (byte cipher [N_BLOCK], byte plain [N_BLOCK])
{
if (round) {
byte s1 [N_BLOCK] ;
copy_and_key (s1, plain, (byte*) (key_sched + round * N_BLOCK)) ;
copy_and_key (s1, cipher, (byte*) (key_sched + round * N_BLOCK)) ;
inv_shift_sub_rows (s1) ;
for (byte r = round ; --r ; ) {
@@ -419,7 +419,7 @@ byte AES::decrypt (byte plain [N_BLOCK], byte cipher [N_BLOCK])
copy_and_key (s2, s1, (byte*) (key_sched + r * N_BLOCK)) ;
inv_mix_sub_columns (s1, s2) ;
}
copy_and_key (cipher, s1, (byte*) (key_sched)) ;
copy_and_key (plain, s1, (byte*) (key_sched)) ;
} else {
return AES_FAILURE ;
}

View File

@@ -30,6 +30,10 @@ const uint8_t sha256InitState[] PROGMEM = {
0x19,0xcd,0xe0,0x5b // H7
};
// Suppress warning about uninitialized variables because initializing them in an init function
// allows the compiler to optimize away the variables in case the class is only instantiated but
// never used.
// cppcheck-suppress uninitMemberVar
Sha256Class::Sha256Class()
{
/*
@@ -165,6 +169,10 @@ uint8_t* Sha256Class::result(void)
#define HMAC_IPAD 0x36
#define HMAC_OPAD 0x5c
// Suppress warning about uninitialized variables because initializing them in an init function
// allows the compiler to optimize away the variables in case the class is only instantiated but
// never used.
// cppcheck-suppress uninitMemberVar
HmacClass::HmacClass()
{
}

View File

@@ -41,6 +41,10 @@ class PinIO
{
public:
/** Create a PinIO object with no assigned pin. */
// Suppress warning about uninitialized variables because initializing them in an init function
// allows the compiler to optimize away the variables in case the class is only instantiated but
// never used.
// cppcheck-suppress uninitMemberVar
PinIO() : bit_(0), mask_(0XFF) {}
/** Constructor
* @param[in] pin Pin assigned to this object.
@@ -186,4 +190,4 @@ private:
volatile uint8_t* portReg_;
};
#endif // PinIO_h
/** @} */
/** @} */

View File

@@ -179,7 +179,6 @@ void dumpAll(void)
//------------------------------------------------------------------------------
void fillNvRam(void)
{
uint8_t buf[8];
PgmPrint("Enter HEX value for all NV RAM locations (00-FF): ");
uint16_t v;
if (!hexRead(&v)) {
@@ -363,4 +362,4 @@ void loop(void)
} else {
PgmPrintln("Invalid option");
}
}
}

View File

@@ -131,13 +131,14 @@ void AltSoftSerial::writeByte(uint8_t b)
ISR(COMPARE_A_INTERRUPT)
{
uint8_t state, byte, bit, head, tail;
uint8_t state, byte, head, tail;
uint16_t target;
state = tx_state;
byte = tx_byte;
target = GET_COMPARE_A();
while (state < 9) {
uint8_t bit;
target += ticks_per_bit;
bit = byte & 1;
byte >>= 1;
@@ -195,9 +196,8 @@ void AltSoftSerial::flushOutput(void)
ISR(CAPTURE_INTERRUPT)
{
uint8_t state, bit, head;
uint16_t capture, target;
int16_t offset;
uint8_t state, bit;
uint16_t capture;
capture = GET_INPUT_CAPTURE();
bit = rx_bit;
@@ -217,8 +217,10 @@ ISR(CAPTURE_INTERRUPT)
rx_state = 1;
}
} else {
uint16_t target;
target = rx_target;
while (1) {
int16_t offset;
offset = capture - target;
if (offset < 0) {
break;
@@ -227,6 +229,7 @@ ISR(CAPTURE_INTERRUPT)
target += ticks_per_bit;
state++;
if (state >= 9) {
uint8_t head;
DISABLE_INT_COMPARE_B();
head = rx_buffer_head + 1;
if (head >= RX_BUFFER_SIZE) {

View File

@@ -113,11 +113,11 @@ unsigned int bcm2835_version(void)
*/
uint32_t bcm2835_peri_read(volatile uint32_t* paddr)
{
uint32_t ret;
if (debug) {
printf("bcm2835_peri_read paddr %08X\n", (unsigned) paddr);
return 0;
} else {
uint32_t ret;
__sync_synchronize();
ret = *paddr;
__sync_synchronize();
@@ -438,7 +438,7 @@ void bcm2835_delayMicroseconds(uint64_t micros)
if (debug) {
/* Cant access sytem timers in debug mode */
printf("bcm2835_delayMicroseconds %lld\n", micros);
printf("bcm2835_delayMicroseconds %" PRIu64 "\n", micros);
return;
}

View File

@@ -30,9 +30,9 @@
// private method to read stream with timeout
int Stream::timedRead()
{
int c;
_startMillis = millis();
do {
int c;
c = read();
if(c >= 0) {
return c;
@@ -45,9 +45,9 @@ int Stream::timedRead()
// private method to peek stream with timeout
int Stream::timedPeek()
{
int c;
_startMillis = millis();
do {
int c;
c = peek();
if(c >= 0) {
return c;
@@ -61,8 +61,8 @@ int Stream::timedPeek()
// discards non-numeric characters
int Stream::peekNextDigit()
{
int c;
while(1) {
int c;
c = timedPeek();
if(c < 0) {
return c; // timeout

View File

@@ -7,6 +7,9 @@
#include "PubSubClient.h"
#include "Arduino.h"
// Suppress uninitialized member variable in all constructors because some memory can be saved with
// on-demand initialization of these members
// cppcheck-suppress uninitMemberVar
PubSubClient::PubSubClient()
{
this->_state = MQTT_DISCONNECTED;
@@ -15,6 +18,7 @@ PubSubClient::PubSubClient()
setCallback(NULL);
}
// cppcheck-suppress uninitMemberVar
PubSubClient::PubSubClient(Client& client)
{
this->_state = MQTT_DISCONNECTED;
@@ -22,6 +26,7 @@ PubSubClient::PubSubClient(Client& client)
this->stream = NULL;
}
// cppcheck-suppress uninitMemberVar
PubSubClient::PubSubClient(IPAddress addr, uint16_t port, Client& client)
{
this->_state = MQTT_DISCONNECTED;
@@ -29,6 +34,7 @@ PubSubClient::PubSubClient(IPAddress addr, uint16_t port, Client& client)
setClient(client);
this->stream = NULL;
}
// cppcheck-suppress uninitMemberVar
PubSubClient::PubSubClient(IPAddress addr, uint16_t port, Client& client, Stream& stream)
{
this->_state = MQTT_DISCONNECTED;
@@ -36,6 +42,8 @@ PubSubClient::PubSubClient(IPAddress addr, uint16_t port, Client& client, Stream
setClient(client);
setStream(stream);
}
// cppcheck-suppress uninitMemberVar
// cppcheck-suppress passedByValue
PubSubClient::PubSubClient(IPAddress addr, uint16_t port, MQTT_CALLBACK_SIGNATURE, Client& client)
{
this->_state = MQTT_DISCONNECTED;
@@ -44,6 +52,8 @@ PubSubClient::PubSubClient(IPAddress addr, uint16_t port, MQTT_CALLBACK_SIGNATUR
setClient(client);
this->stream = NULL;
}
// cppcheck-suppress uninitMemberVar
// cppcheck-suppress passedByValue
PubSubClient::PubSubClient(IPAddress addr, uint16_t port, MQTT_CALLBACK_SIGNATURE, Client& client,
Stream& stream)
{
@@ -54,6 +64,7 @@ PubSubClient::PubSubClient(IPAddress addr, uint16_t port, MQTT_CALLBACK_SIGNATUR
setStream(stream);
}
// cppcheck-suppress uninitMemberVar
PubSubClient::PubSubClient(uint8_t *ip, uint16_t port, Client& client)
{
this->_state = MQTT_DISCONNECTED;
@@ -61,6 +72,7 @@ PubSubClient::PubSubClient(uint8_t *ip, uint16_t port, Client& client)
setClient(client);
this->stream = NULL;
}
// cppcheck-suppress uninitMemberVar
PubSubClient::PubSubClient(uint8_t *ip, uint16_t port, Client& client, Stream& stream)
{
this->_state = MQTT_DISCONNECTED;
@@ -68,6 +80,8 @@ PubSubClient::PubSubClient(uint8_t *ip, uint16_t port, Client& client, Stream& s
setClient(client);
setStream(stream);
}
// cppcheck-suppress uninitMemberVar
// cppcheck-suppress passedByValue
PubSubClient::PubSubClient(uint8_t *ip, uint16_t port, MQTT_CALLBACK_SIGNATURE, Client& client)
{
this->_state = MQTT_DISCONNECTED;
@@ -76,6 +90,8 @@ PubSubClient::PubSubClient(uint8_t *ip, uint16_t port, MQTT_CALLBACK_SIGNATURE,
setClient(client);
this->stream = NULL;
}
// cppcheck-suppress uninitMemberVar
// cppcheck-suppress passedByValue
PubSubClient::PubSubClient(uint8_t *ip, uint16_t port, MQTT_CALLBACK_SIGNATURE, Client& client,
Stream& stream)
{
@@ -86,6 +102,7 @@ PubSubClient::PubSubClient(uint8_t *ip, uint16_t port, MQTT_CALLBACK_SIGNATURE,
setStream(stream);
}
// cppcheck-suppress uninitMemberVar
PubSubClient::PubSubClient(const char* domain, uint16_t port, Client& client)
{
this->_state = MQTT_DISCONNECTED;
@@ -93,6 +110,7 @@ PubSubClient::PubSubClient(const char* domain, uint16_t port, Client& client)
setClient(client);
this->stream = NULL;
}
// cppcheck-suppress uninitMemberVar
PubSubClient::PubSubClient(const char* domain, uint16_t port, Client& client, Stream& stream)
{
this->_state = MQTT_DISCONNECTED;
@@ -100,6 +118,8 @@ PubSubClient::PubSubClient(const char* domain, uint16_t port, Client& client, St
setClient(client);
setStream(stream);
}
// cppcheck-suppress uninitMemberVar
// cppcheck-suppress passedByValue
PubSubClient::PubSubClient(const char* domain, uint16_t port, MQTT_CALLBACK_SIGNATURE,
Client& client)
{
@@ -109,6 +129,8 @@ PubSubClient::PubSubClient(const char* domain, uint16_t port, MQTT_CALLBACK_SIGN
setClient(client);
this->stream = NULL;
}
// cppcheck-suppress uninitMemberVar
// cppcheck-suppress passedByValue
PubSubClient::PubSubClient(const char* domain, uint16_t port, MQTT_CALLBACK_SIGNATURE,
Client& client, Stream& stream)
{
@@ -337,14 +359,13 @@ bool PubSubClient::loop()
if (_client->available()) {
uint8_t llen;
uint16_t len = readPacket(&llen);
uint16_t msgId = 0;
uint8_t *payload;
if (len > 0) {
lastInActivity = t;
uint8_t type = buffer[0]&0xF0;
if (type == MQTTPUBLISH) {
if (callback) {
uint16_t tl = (buffer[llen+1]<<8)+buffer[llen+2];
uint8_t *payload;
char topic[tl+1];
for (uint16_t i=0; i<tl; i++) {
topic[i] = buffer[llen+3+i];
@@ -352,7 +373,7 @@ bool PubSubClient::loop()
topic[tl] = 0;
// msgId only present for QOS>0
if ((buffer[0]&0x06) == MQTTQOS1) {
msgId = (buffer[llen+3+tl]<<8)+buffer[llen+3+tl+1];
uint16_t msgId = (buffer[llen+3+tl]<<8)+buffer[llen+3+tl+1];
payload = buffer+llen+3+tl+2;
callback(topic,payload,len-llen-3-tl-2);
@@ -424,8 +445,6 @@ bool PubSubClient::publish(const char* topic, const uint8_t* payload, unsigned i
bool PubSubClient::publish_P(const char* topic, const uint8_t* payload, unsigned int plength,
bool retained)
{
uint8_t llen = 0;
uint8_t digit;
unsigned int rc = 0;
uint16_t tlen;
unsigned int pos = 0;
@@ -446,13 +465,13 @@ bool PubSubClient::publish_P(const char* topic, const uint8_t* payload, unsigned
buffer[pos++] = header;
len = plength + 2 + tlen;
do {
uint8_t digit;
digit = len % 128;
len = len / 128;
if (len > 0) {
digit |= 0x80;
}
buffer[pos++] = digit;
llen++;
} while(len>0);
pos = writeString(topic,buffer,pos);
@@ -472,11 +491,11 @@ bool PubSubClient::write(uint8_t header, uint8_t* buf, uint16_t length)
{
uint8_t lenBuf[4];
uint8_t llen = 0;
uint8_t digit;
uint8_t pos = 0;
uint16_t rc;
uint16_t len = length;
do {
uint8_t digit;
digit = len % 128;
len = len / 128;
if (len > 0) {
@@ -626,6 +645,7 @@ PubSubClient& PubSubClient::setServer(const char * domain, uint16_t port)
return *this;
}
// cppcheck-suppress passedByValue
PubSubClient& PubSubClient::setCallback(MQTT_CALLBACK_SIGNATURE)
{
this->callback = callback;

View File

@@ -103,7 +103,7 @@ private:
int _state;
public:
PubSubClient(); //!< PubSubClient
PubSubClient(Client& client); //!< PubSubClient
explicit PubSubClient(Client& client); //!< PubSubClient
PubSubClient(IPAddress, uint16_t, Client& client); //!< PubSubClient
PubSubClient(IPAddress, uint16_t, Client& client, Stream&); //!< PubSubClient
PubSubClient(IPAddress, uint16_t, MQTT_CALLBACK_SIGNATURE,Client& client); //!< PubSubClient

View File

@@ -47,6 +47,9 @@ uint8_t SPIFlash::UNIQUEID[8];
/// get this from the datasheet of your flash chip
/// Example for Atmel-Adesto 4Mbit AT25DF041A: 0x1F44 (page 27: http://www.adestotech.com/sites/default/files/datasheets/doc3668.pdf)
/// Example for Winbond 4Mbit W25X40CL: 0xEF30 (page 14: http://www.winbond.com/NR/rdonlyres/6E25084C-0BFE-4B25-903D-AE10221A0929/0/W25X40CL.pdf)
// Suppress uninitialized member variable in constructor because some memory can be saved with
// on-demand initialization of these members
// cppcheck-suppress uninitMemberVar
SPIFlash::SPIFlash(uint8_t slaveSelectPin, uint16_t jedecID)
{
_slaveSelectPin = slaveSelectPin;
@@ -248,7 +251,6 @@ void SPIFlash::writeBytes(uint32_t addr, const void* buf, uint16_t len)
//SST25 Type of Flash does not support Page Programming but AAI Word Programming
uint16_t i=0;
uint8_t oddAdr=0;
char s[5];
command(SPIFLASH_AAIWORDPROGRAM, true); // Byte/Page Program
SPI.transfer(addr >> 16);

View File

@@ -127,13 +127,13 @@ void _serialReset()
// function.
bool _serialProcess()
{
char inch;
unsigned char i;
if (!_dev.available()) {
return false;
}
while(_dev.available()) {
char inch;
inch = _dev.read();
switch(_recPhase) {
@@ -239,7 +239,6 @@ bool transportSend(const uint8_t to, const void* data, const uint8_t len, const
const char *datap = static_cast<char const *>(data);
unsigned char i;
unsigned char cs = 0;
unsigned char del;
// This is how many times to try and transmit before failing.
unsigned char timeout = 10;
@@ -248,6 +247,7 @@ bool transportSend(const uint8_t to, const void* data, const uint8_t len, const
// the last millisecond, then wait for a random time and check again.
while (_serialProcess()) {
unsigned char del;
del = rand() % 20;
for (i = 0; i < del; i++) {
delay(1);

View File

@@ -57,8 +57,9 @@
"args": ["--force", "--library=avr",
"--suppressions-list=$project_path/../../.mystools/cppcheck/config/suppressions.cfg",
"-I$project_path/../../", "-I$project_path/../../core",
"-DMY_SIGNING_NODE_WHITELISTING",
"-DMY_SIGNING_SIMPLE_PASSWD"],
// Arduino IDE hardware location might differ on your system...
"-I$project_path/../../../../../Apps/arduino/hardware/tools/avr/avr/include",
"-DCPPCHECK", "--language=c++", "--inline-suppr"],
"enable": "style,information",
"excludes": [],
"std": []