Skip to content

WIP: Fixed various warnings and errors identified by cppcheck #810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jimklimov merged 6 commits into from
Nov 4, 2020
Merged

WIP: Fixed various warnings and errors identified by cppcheck #810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1b6a214
WIP: Fixed various warnings and errors identified by cppcheck
seanm Aug 8, 2020
c2329b0
Merge branch 'master' into cppcheck-fixes
jimklimov Oct 9, 2020
7a02f41
Merge branch 'master' into cppcheck-fixes
jimklimov Oct 9, 2020
cb02002
Update nutdrv_qx_voltronic.c
jimklimov Nov 4, 2020
dfb4dae
Update solis.c
jimklimov Nov 4, 2020
e735db6
Merge branch 'master' into cppcheck-fixes
jimklimov Nov 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clients/cgilib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static char *unescape(char *buf)
hex[1] = buf[++i];
hex[2] = '\0';
if (!isxdigit((unsigned char) hex[0])
|| !isxdigit((unsigned char) hex[0]))
|| !isxdigit((unsigned char) hex[1]))
fatalx(EXIT_FAILURE, "bad escape char");
ch = strtol(hex, NULL, 16);

Expand Down
4 changes: 3 additions & 1 deletion clients/nutclient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,13 +505,15 @@ TcpClient::TcpClient():
Client(),
_host("localhost"),
_port(3493),
_timeout(0),
_socket(new internal::Socket)
{
// Do not connect now
}

TcpClient::TcpClient(const std::string& host, int port):
Client(),
_timeout(0),
_socket(new internal::Socket)
{
connect(host, port);
Expand Down Expand Up @@ -900,7 +902,7 @@ std::string TcpClient::sendQuery(const std::string& req)

void TcpClient::sendAsyncQueries(const std::vector<std::string>& req)
{
for (std::vector<std::string>::const_iterator it = req.cbegin(); it != req.cend(); it++)
for (std::vector<std::string>::const_iterator it = req.cbegin(); it != req.cend(); ++it)
{
_socket->write(*it);
}
Expand Down
5 changes: 1 addition & 4 deletions clients/upsmon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,10 +944,7 @@ static void addups(int reloading, const char *sys, const char *pvs,
/* build this up so the user doesn't run with bad settings */
totalpv += tmp->pv;

if (un)
tmp->un = xstrdup(un);
else
tmp->un = NULL;
tmp->un = xstrdup(un);

tmp->pw = xstrdup(pw);
tmp->status = 0;
Expand Down
5 changes: 3 additions & 2 deletions clients/upsset.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ static void showcmds(void)
/* CMD upsname cmdname */
if (numa < 3) {
fprintf(stderr, "Error: insufficient data "
"(got %d args, need at least 3)\n", numa);
"(got %u args, need at least 3)\n", numa);

return;
}
Expand Down Expand Up @@ -661,6 +661,7 @@ static void do_enum(const char *varname)
printf("Unavailable\n");
fprintf(stderr, "Error doing ENUM %s %s: %s\n",
upsname, varname, upscli_strerror(&ups));
free(val);
return;
}

Expand All @@ -674,7 +675,7 @@ static void do_enum(const char *varname)

if (numa < 4) {
fprintf(stderr, "Error: insufficient data "
"(got %d args, need at least 4)\n", numa);
"(got %u args, need at least 4)\n", numa);

free(val);
return;
Expand Down
2 changes: 1 addition & 1 deletion drivers/al175.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ static int al_parse_reply(io_head_t *io_head, raw_data_t *io_buf, /*const*/ raw_

int err;
unsigned i;
const byte_t *reply = raw_reply.begin - 1;
const byte_t *reply = NULL;

/* 1: extract header and parse it */
/*const*/ raw_data_t raw_reply_head = raw_reply;
Expand Down
2 changes: 1 addition & 1 deletion drivers/apcsmart-old.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ static int (*sdlist[])(int) = {

static void upsdrv_shutdown_simple(int status)
{
unsigned int sdtype = 0;
long sdtype = 0;
char *strval;

if ((strval = getval("sdtype"))) {
Expand Down
2 changes: 1 addition & 1 deletion drivers/asem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ void upsdrv_updateinfo(void)

status_init();
status_set(online ? "OL" : "OB");
if (!discharging & !fully_charged)
if (!discharging && !fully_charged)
status_set("CHRG");
else if (discharging && current < 0)
status_set("DISCHRG");
Expand Down
2 changes: 1 addition & 1 deletion drivers/bcmxcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ bool_t init_command(int size)
for (ncounter = 0; ncounter < NumComms; ncounter++)
{
commandByte = answer[iIndex];
if(commandByte >= 0 && commandByte < BCMXCP_COMMAND_MAP_MAX) {
if(commandByte < BCMXCP_COMMAND_MAP_MAX) {
upsdebugx(2, "%03d\t%02x\t%s", ncounter, commandByte, bcmxcp_command_map[commandByte].command_desc);
bcmxcp_command_map[commandByte].command_byte = commandByte;
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/belkinunv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ void upsdrv_initinfo(void)

val = belkin_nut_read_int(REG_ALARMSTATUS);
if (val!=-1) {
dstate_setinfo("ups.beeper.status", "%s", val==1 ? "disabled" : val&1 ? "muted" : "enabled");
dstate_setinfo("ups.beeper.status", "%s", (val==1) ? "disabled" : (val&1) ? "muted" : "enabled");

/* declare variable writable */
dstate_setflags("ups.beeper.status", ST_FLAG_RW);
Expand Down Expand Up @@ -1060,7 +1060,7 @@ void upsdrv_updateinfo(void)

val = belkin_nut_read_int(REG_ALARMSTATUS);
if (val!=-1) {
dstate_setinfo("ups.beeper.status", "%s", val==1 ? "disabled" : val&1 ? "muted" : "enabled");
dstate_setinfo("ups.beeper.status", "%s", (val==1) ? "disabled" : (val&1) ? "muted" : "enabled");
}

val = belkin_nut_read_int(REG_SHUTDOWNTIMER);
Expand Down
2 changes: 1 addition & 1 deletion drivers/dstate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1167,7 +1167,7 @@ int dstate_detect_phasecount(

if ( (v1 && v2 && !v3) ||
(v1n && v2n && !v3n) ||
(c1 && c2 && !c2) ||
(c1 && c2 && !c3) ||
(v12 && !v23 && !v31) ) {
upsdebugx(5, "%s(): determined a 2-phase case", __func__);
*num_phases = 2;
Expand Down
7 changes: 3 additions & 4 deletions drivers/gamatronic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,9 @@ int sec_cmd(const char mode, const char *command, char *msgbuf, int *buflen)

if (ret < 0) return -1;

if (ret >= 0) {
strncpy(msgbuf, msg, ret);
upsdebugx(1, "UPS<--PC: \"%s\"",msg);
}
strncpy(msgbuf, msg, ret);
upsdebugx(1, "UPS<--PC: \"%s\"",msg);

/* *(msgbuf+ret) = '\0';*/

*buflen = ret;
Expand Down
2 changes: 1 addition & 1 deletion drivers/isbmex.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ static const char *getpacket(int *we_know){
}

r=read(upsfd,buf,255);
D(printf("%d bytes read: ",r);)
D(printf("%zd bytes read: ",r);)
buf[r]=0;
if (bytes_per_packet && r < bytes_per_packet){
ssize_t rr;
Expand Down
20 changes: 9 additions & 11 deletions drivers/mge-utalk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,19 +896,17 @@ static int mge_command(char *reply, int replylen, const char *fmt, ...)
}

/* send terminating string */
if (MGE_COMMAND_ENDCHAR) {
for (p = MGE_COMMAND_ENDCHAR; *p; p++) {
if ( isprint(*p & 0xFF) )
upsdebugx(4, "mge_command: sending [%c]", *p);
else
upsdebugx(4, "mge_command: sending [%02X]", *p);
for (p = MGE_COMMAND_ENDCHAR; *p; p++) {
if ( isprint(*p & 0xFF) )
upsdebugx(4, "mge_command: sending [%c]", *p);
else
upsdebugx(4, "mge_command: sending [%02X]", *p);

if (write(upsfd, p, 1) != 1)
return -1;
if (write(upsfd, p, 1) != 1)
return -1;

bytes_sent++;
usleep(MGE_CHAR_DELAY);
}
bytes_sent++;
usleep(MGE_CHAR_DELAY);
}

if ( !reply )
Expand Down
4 changes: 2 additions & 2 deletions drivers/nut-libfreeipmi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ int nut_ipmi_get_sensors_status(IPMIDevice_t *ipmi_dev)
* if ((sensor_state = ipmi_monitoring_sensor_read_sensor_state (mon_ctx)) < 0)
* ... */

if ((sensor_reading = ipmi_monitoring_sensor_read_sensor_reading (mon_ctx)) < 0)
if ((sensor_reading = ipmi_monitoring_sensor_read_sensor_reading (mon_ctx)) == NULL)
{
upsdebugx (1, "ipmi_monitoring_sensor_read_sensor_reading() error: %s",
ipmi_monitoring_ctx_errormsg (mon_ctx));
Expand All @@ -881,7 +881,7 @@ int nut_ipmi_get_sensors_status(IPMIDevice_t *ipmi_dev)
continue;
}

if ((sensor_bitmask_strings = ipmi_monitoring_sensor_read_sensor_bitmask_strings (mon_ctx)) < 0)
if ((sensor_bitmask_strings = ipmi_monitoring_sensor_read_sensor_bitmask_strings (mon_ctx)) == NULL)
{
upsdebugx (1, "ipmi_monitoring_sensor_read_sensor_bitmask_strings() error: %s",
ipmi_monitoring_ctx_errormsg (mon_ctx));
Expand Down
2 changes: 1 addition & 1 deletion drivers/nutdrv_qx_voltronic-qs-hex.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ static int voltronic_qs_hex_preprocess_qs_answer(item_t *item, const int len)
}

/* 'Unescape' raw data */
if (item->answer[i] == 0x28 && i < len) {
if (i < len && item->answer[i] == 0x28) {

switch (item->answer[i + 1])
{
Expand Down
6 changes: 3 additions & 3 deletions drivers/nutdrv_qx_voltronic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ static int voltronic_bypass_volt_max(char *value, const size_t len)
if (ivn >= 200)
return -1;

if (protocol == 2 || protocol == 2 || protocol == 10 || protocol == 13 || protocol == 14)
if (protocol == 2 || protocol == 3 || protocol == 10 || protocol == 13 || protocol == 14)
return 0;

break;
Expand Down Expand Up @@ -300,7 +300,7 @@ static int voltronic_bypass_volt_max(char *value, const size_t len)
if (ivn < 200)
return -1;

if (protocol == 2 || protocol == 2 || protocol == 10 || protocol == 13 || protocol == 14 || protocol == 99)
if (protocol == 2 || protocol == 3 || protocol == 10 || protocol == 13 || protocol == 14 || protocol == 99)
Copy link
Member

@jimklimov jimklimov Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aquette @zykh @clepple : experts wanted here :)

Copy link
Member

@aquette aquette Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto above @zykh

return 0;

break;
Expand Down Expand Up @@ -394,7 +394,7 @@ static int voltronic_bypass_volt_min(char *value, const size_t len)
if (ivn >= 200)
return -1;

if (protocol == 2 || protocol == 3 || protocol == 10 || protocol == 10 || protocol == 13 || protocol == 14)
if (protocol == 2 || protocol == 3 || protocol == 10 || protocol == 13 || protocol == 14)
Copy link
Member

@jimklimov jimklimov Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aquette @zykh @clepple : experts wanted here :)

Copy link
Member

@aquette aquette Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto above @zykh

return 0;

break;
Expand Down
2 changes: 1 addition & 1 deletion drivers/oneac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ void EliminateLeadingZeroes (const char* buff1, int StringSize, char* buff2,
memset(buff2, '\0', buff2size); /* Fill with nulls */

/* Find first non-'0' */
while ((buff1[i] == '0') && (i < (StringSize - 1)))
while ((i < (StringSize - 1) && (buff1[i] == '0')))
{
i++;
}
Expand Down
8 changes: 4 additions & 4 deletions drivers/powercom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ static float load_level(void)
} else if ( !strcmp(types[type].name, "IMP") || !strcmp(types[type].name, "OPTI")) {
return raw_data[UPS_LOAD];
}
return raw_data[STATUS_A] & MAINS_FAILURE ?
return (raw_data[STATUS_A] & MAINS_FAILURE) ?
types[type].loadpct[0] * raw_data[UPS_LOAD] +
types[type].loadpct[1] :
types[type].loadpct[2] * raw_data[UPS_LOAD] +
Expand All @@ -693,9 +693,9 @@ static float batt_level(void)
battval=(raw_data[UPS_LOAD])/4+raw_data[BATTERY_CHARGE];
if (battval<=bat0)
return 0.0;
if (battval>bat0 && battval<=bat29)
if (battval<=bat29)
return (battval-bat0)*30.0/(bat29-bat0);
if (battval>bat29 && battval<=bat100)
if (battval<=bat100)
return 30.0+(battval-bat29)*70.0/(bat100-bat29);
return 100.0;
}
Expand Down Expand Up @@ -729,7 +729,7 @@ static float batt_level(void)
}
if ( !strcmp(types[type].name, "IMP") || !strcmp(types[type].name, "OPTI"))
return raw_data[BATTERY_CHARGE];
return raw_data[STATUS_A] & ONLINE ? /* Are we on battery power? */
return (raw_data[STATUS_A] & ONLINE) ? /* Are we on battery power? */
/* Yes */
types[type].battpct[0] * raw_data[BATTERY_CHARGE] +
types[type].battpct[1] * load_level() + types[type].battpct[2] :
Expand Down
2 changes: 1 addition & 1 deletion drivers/rhino.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ AutonomyCalc( int ia ) /* all models */
{
currin = ( UtilPowerOut + ConstInt ) *1.0 / Vin;
auton = ( ( ( AmpH *1.0 / currin ) * 60 * ( ( BattVoltage - VbatMin ) * 1.0 /( VbatNom - VbatMin ) ) * FM ) + FA );
if( ( BattVoltage > 129 ) || ( BattVoltage < 144 ) )
if( ( BattVoltage > 129 ) && ( BattVoltage < 144 ) )
result = 133;
else
result = (int) auton;
Expand Down
1 change: 0 additions & 1 deletion drivers/riello_ser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ static int char_read (char *bytes, int size, int read_timeout)
return -1;
}
else {
bytes += now;
Copy link
Member

@jimklimov jimklimov Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this got inherited from some loop where the bytes pointer got incremented so we read next portions into the buffer while we can. The read(..., size - readen) above points to this too - with the current code, readen is always zero until we increment it with now.

Probably the current routine can be simplified even further to drop readen altogether, return now here and return 0 in default exit below. But I do not know the legacy context behind it... @aquette @zykh @clepple : any memories on this one?

Copy link
Contributor Author

@seanm seanm Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed this looks leftover from some old refactoring. I'm inclined not to attempt any simplifications here.

readen += now;
}
}
Expand Down
2 changes: 1 addition & 1 deletion drivers/snmp-ups.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1559,7 +1559,7 @@ bool_t is_multiple_template(const char *OID_template)
* Note: remember to adapt info_type, OID and optionaly dfl */
snmp_info_t *instantiate_info(snmp_info_t *info_template, snmp_info_t *new_instance)
{
upsdebugx(1, "%s(%s)", __func__, info_template->info_type);
upsdebugx(1, "%s(%s)", __func__, info_template ? info_template->info_type : "n/a");

/* sanity check */
if (info_template == NULL)
Expand Down
5 changes: 2 additions & 3 deletions drivers/solis.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ static void scan_received_pack(void) {
if (!SourceFail && InversorOn)
OutFreq = InFreq;

if (AppPower <= 0) /* charge pf */
if (AppPower < 0) /* charge pf */
ChargePowerFactor = 0;
else {
if( AppPower == 0 )
Expand Down Expand Up @@ -728,8 +728,7 @@ static void get_base_info(void) {
} else {
if (i2 == 1 && DaysOffWeek > 0) {
isprogram = 1;
if (DaysOnWeek != DaysOffWeek)
DaysOnWeek = DaysOffWeek;
DaysOnWeek = DaysOffWeek;
}
}
} /* end prgups 1 - 2 */
Expand Down
2 changes: 1 addition & 1 deletion drivers/tripplite_usb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ static int send_cmd(const unsigned char *msg, size_t msg_len, unsigned char *rep
}

#if ! defined(__FreeBSD__)
if(!done) { usleep(1000*100); /* TODO: nanosleep */ }
usleep(1000*100); /* TODO: nanosleep */
#endif

for(recv_try=0; !done && recv_try < MAX_RECV_TRIES; recv_try++) {
Expand Down
2 changes: 1 addition & 1 deletion drivers/tripplitesu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ static void set_sensitivity(const char *val) {

for (i = 0; i < sizeof(sensitivity) / sizeof(sensitivity[0]); i++) {
if (!strcasecmp(val, sensitivity[i].name)) {
snprintf(parm, sizeof(parm), "%d", i);
snprintf(parm, sizeof(parm), "%u", i);
do_command(SET, VOLTAGE_SENSITIVITY, parm, NULL);
break;
}
Expand Down
2 changes: 1 addition & 1 deletion server/netssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ void ssl_init(void)

int ssl_read(nut_ctype_t *client, char *buf, size_t buflen)
{
int ret;
int ret = -1;

if (!client->ssl_connected) {
return -1;
Expand Down
26 changes: 12 additions & 14 deletions tools/nut-scanner/scan_nut.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,24 +177,22 @@ static void * list_nut_devices(void * arg)
/* FIXME:
* - also print answer[2] if != "Unavailable"?
* - for upsmon.conf or ups.conf (using dummy-ups)? */
if (numa >= 3) {
dev = nutscan_new_device();
dev->type = TYPE_NUT;
dev->driver = strdup("nutclient");
/* +1+1 is for '@' character and terminating 0 */
buf_size = strlen(answer[1])+strlen(hostname)+1+1;
dev->port = malloc(buf_size);
if( dev->port ) {
snprintf(dev->port,buf_size,"%s@%s",answer[1],
hostname);
dev = nutscan_new_device();
dev->type = TYPE_NUT;
dev->driver = strdup("nutclient");
/* +1+1 is for '@' character and terminating 0 */
buf_size = strlen(answer[1])+strlen(hostname)+1+1;
dev->port = malloc(buf_size);
if( dev->port ) {
snprintf(dev->port,buf_size,"%s@%s",answer[1],
hostname);
#ifdef HAVE_PTHREAD
pthread_mutex_lock(&dev_mutex);
pthread_mutex_lock(&dev_mutex);
#endif
dev_ret = nutscan_add_device_to_device(dev_ret,dev);
dev_ret = nutscan_add_device_to_device(dev_ret,dev);
#ifdef HAVE_PTHREAD
pthread_mutex_unlock(&dev_mutex);
pthread_mutex_unlock(&dev_mutex);
#endif
}

}
}
Expand Down