Reconcile int-ish data types with formatting strings [#823]

New checks in clang-18/19 brought new findings, some of them reasonable :)

* "%x" style formatting requires an (unsigned int) variable
* numeric literals and macros are (int) by default
* results of math with (uint16_t), done in some cases, are upscaled into
  (int) by default
* char, unsigned or not, seems to be also upscaled into (int)

Overall, had to change formatting strings in some cases, variable types
in others (e.g. flags or notification types do not make sense as signed)
and added casting in a few places that remained.

Signed-off-by: Jim Klimov <[email protected]>

Author: jimklimov

NEWS.adoc

@@ -457,6 +457,16 @@ relocated into new `shutdown.default` INSTCMD definitions. [#2670]
      `gcc-13`+ whose static analyzers on NUT CI farm complained about some
      imperfections after adding newer OS revisions to the population of
      build agents. [#2585, #2588]
+   * New checks in `clang-19` brought new findings about mismatched formatting
+     strings and `int`-ish parameters of respective methods.
+     Overall, had to change formatting strings in some cases, variable types
+     in others (e.g. flags or notification types do not make sense as signed)
+     and added casting in a few places that remained, because:
+     - `%x` style formatting requires an `unsigned int` variable
+     - numeric literals and macros are `int` by default
+     - results of math with unsigned types like `uint16_t`, done in some
+       cases, are up-scaled into `int` by default
+     - `char`'s, `unsigned` or not, seem to be also up-scaled into `int`
 
  - updated `docs/nut-names.txt` with items defined by 42ITy NUT fork. [#2339]
 

clients/upscmd.c

@@ -61,7 +61,7 @@ static void usage(const char *prog)
 	printf("  -p <password>	set password for command authentication\n");
 	printf("  -w            wait for the completion of command by the driver\n");
 	printf("                and return its actual result from the device\n");
-	printf("  -t <timeout>	set a timeout when using -w (in seconds, default: %u)\n", DEFAULT_TRACKING_TIMEOUT);
+	printf("  -t <timeout>	set a timeout when using -w (in seconds, default: %d)\n", DEFAULT_TRACKING_TIMEOUT);
 	printf("\n");
 	printf("  <ups>		UPS identifier - <upsname>[@<hostname>[:<port>]]\n");
 	printf("  <command>	Valid instant command - test.panel.start, etc.\n");

clients/upslog.c

@@ -351,7 +351,7 @@ static void do_etime(const char *arg)
 	NUT_UNUSED_VARIABLE(arg);
 
 	time(&tod);
-	snprintfcat(logbuffer, sizeof(logbuffer), "%ld", (unsigned long) tod);
+	snprintfcat(logbuffer, sizeof(logbuffer), "%lu", (unsigned long) tod);
 }
 
 static void print_literal(const char *arg)

clients/upsmon.c

@@ -290,7 +290,7 @@ static unsigned __stdcall async_notify(LPVOID param)
 }
 #endif
 
-static void notify(const char *notice, int flags, const char *ntype,
+static void notify(const char *notice, unsigned int flags, const char *ntype,
 			const char *upsname)
 {
 #ifndef WIN32
@@ -380,7 +380,7 @@ static void notify(const char *notice, int flags, const char *ntype,
 #endif
 }
 
-static void do_notify(const utype_t *ups, int ntype, const char *extra)
+static void do_notify(const utype_t *ups, unsigned int ntype, const char *extra)
 {
 	int	i;
 	char	msg[SMALLBUF], *upsname = NULL;
@@ -394,7 +394,8 @@ static void do_notify(const utype_t *ups, int ntype, const char *extra)
 			const char	*msgfmt = (notifylist[i].msg ? notifylist[i].msg : notifylist[i].stockmsg);
 			char	*s = strstr(msgfmt, "%s");
 
-			upsdebugx(2, "%s: ntype 0x%04x (%s)", __func__, ntype,
+			upsdebugx(2, "%s: ntype 0x%04x (%s)",
+				__func__, ntype,
 				notifylist[i].name);
 #ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_NONLITERAL
 #pragma GCC diagnostic push
@@ -1371,8 +1372,8 @@ static int is_ups_critical(utype_t *ups)
 				"%s: seems that UPS [%s] is OB+LB now, "
 				"but OBLBDURATION=%d - not declaring "
 				"a critical state just now, but will "
-				"bump the polling rate to %d (from "
-				"POLLFREQALERT=%d) and wait to see "
+				"bump the polling rate to %u (from "
+				"POLLFREQALERT=%u) and wait to see "
 				"if this situation dissipates soon",
 				__func__, ups->upsname,
 				oblbdurationtime,
@@ -1604,7 +1605,7 @@ static void redefine_ups(utype_t *ups, unsigned int pv, const char *un,
 		 * from seeing it again? This is a somewhat rare case
 		 * of self-inflicted pain, if it does even happen :)
 		 */
-		upslogx(LOG_INFO, "UPS [%s]: redefined power value to %d",
+		upslogx(LOG_INFO, "UPS [%s]: redefined power value to %u",
 			ups->sys, pv);
 		ups->pv = pv;
 	}
@@ -1818,7 +1819,7 @@ static void addups(int reloading, const char *sys, const char *pvs,
 	/* Negative debug value may be set by help() to be really quiet */
 	if (nut_debug_level > -1) {
 		if (tmp->pv)
-			upslogx(LOG_INFO, "UPS: %s (%s) (power value %d)", tmp->sys,
+			upslogx(LOG_INFO, "UPS: %s (%s) (power value %u)", tmp->sys,
 				flag_isset(tmp->status, ST_PRIMARY) ? "primary" : "secondary",
 				tmp->pv);
 		else
@@ -2835,7 +2836,7 @@ static void pollups(utype_t *ups)
 				upslogx(LOG_ERR, "Poll UPS [%s] failure state code "
 					"changed from %d to %d; "
 					"report below will only be repeated to syslog "
-					"every %d polling loop cycles (%d sec):",
+					"every %d polling loop cycles (%u sec):",
 					ups->sys, ups->pollfail_log_throttle_state,
 					upserror, pollfail_log_throttle_max,
 					pollfail_log_throttle_max * pollfreq);
@@ -3202,8 +3203,8 @@ static void reload_conf(void)
 
 	/* see if the user just blew off a foot */
 	if (totalpv < minsupplies) {
-		upslogx(LOG_CRIT, "Fatal error: total power value (%d) less "
-			"than MINSUPPLIES (%d)", totalpv, minsupplies);
+		upslogx(LOG_CRIT, "Fatal error: total power value (%u) less "
+			"than MINSUPPLIES (%u)", totalpv, minsupplies);
 
 		fatalx(EXIT_FAILURE, "Impossible power configuration, unable to continue");
 	}
@@ -3577,8 +3578,8 @@ int main(int argc, char *argv[])
 	if (totalpv < minsupplies) {
 		printf("\nFatal error: insufficient power configured!\n\n");
 
-		printf("Sum of power values........: %d\n", totalpv);
-		printf("Minimum value (MINSUPPLIES): %d\n", minsupplies);
+		printf("Sum of power values........: %u\n", totalpv);
+		printf("Minimum value (MINSUPPLIES): %u\n", minsupplies);
 
 		printf("\nEdit your upsmon.conf and change the values.\n");
 		exit(EXIT_FAILURE);

clients/upsmon.h

@@ -149,7 +149,7 @@ typedef struct {
  * To move or not to move?..
  */
 static struct {
-	int	type;
+	unsigned int	type;
 	const	char	*name;
 	char	*msg;	/* NULL until overridden */
 	const char	*stockmsg;

clients/upsrw.c

@@ -62,7 +62,7 @@ static void usage(const char *prog)
 	printf("  -p <password> set password for command authentication\n");
 	printf("  -w            wait for the completion of setting by the driver\n");
 	printf("                and return its actual result from the device\n");
-	printf("  -t <timeout>	set a timeout when using -w (in seconds, default: %u)\n", DEFAULT_TRACKING_TIMEOUT);
+	printf("  -t <timeout>	set a timeout when using -w (in seconds, default: %d)\n", DEFAULT_TRACKING_TIMEOUT);
 	printf("\n");
 	printf("  <ups>         UPS identifier - <upsname>[@<hostname>[:<port>]]\n");
 	printf("\n");

common/common.c

@@ -4298,7 +4298,7 @@ int match_regex_hex(const regex_t *preg, const int n)
 {
 	char	buf[10];
 
-	snprintf(buf, sizeof(buf), "%04x", n);
+	snprintf(buf, sizeof(buf), "%04x", (unsigned int)n);
 
 	return match_regex(preg, buf);
 }

common/parseconf.c

@@ -184,7 +184,7 @@ static void addchar(PCONF_CTX_t *ctx)
 	/* CVE-2012-2944: only allow the subset of ASCII charset from Space to ~ */
 	if ((ctx->ch < 0x20) || (ctx->ch > 0x7f)) {
 		fprintf(stderr, "addchar: discarding invalid character (0x%02x)!\n",
-				ctx->ch);
+				(unsigned int)ctx->ch);
 		return;
 	}
 

drivers/adelsystem_cbi.c

@@ -592,12 +592,12 @@ void reginit(void)
 			 */
 			default:
 				upslogx(LOG_ERR,
-					"Invalid register type %d for register %d",
+					"Invalid register type %u for register %d",
 					regs[i].type,
 					regs[i].num
 				);
 				upsdebugx(3,
-					"Invalid register type %d for register %d",
+					"Invalid register type %u for register %d",
 					regs[i].type,
 					regs[i].num
 				);
@@ -609,11 +609,11 @@ void reginit(void)
 #endif
 		}
 		upsdebugx(3,
-			"reginit: num:%d, type: %d saddr: %d, xaddr: 0x%x",
+			"reginit: num:%d, type: %u saddr: %d, xaddr: 0x%x",
 			regs[i].num,
 			regs[i].type,
 			regs[i].saddr,
-			regs[i].xaddr
+			(unsigned int)(regs[i].xaddr)
 		);
 	}
 }
@@ -627,9 +627,9 @@ int read_all_regs(modbus_t *mb, uint16_t *data)
 	rval = modbus_read_registers(mb, regs[H_REG_STARTIDX].xaddr, MAX_H_REGS, data);
 	if (rval == -1) {
 		upslogx(LOG_ERR,
-			"ERROR:(%s) modbus_read: addr:0x%x, length:%8d, path:%s\n",
+			"ERROR:(%s) modbus_read: addr:0x%x, length:%8d, path:%s",
 			modbus_strerror(errno),
-			regs[H_REG_STARTIDX].xaddr,
+			(unsigned int)(regs[H_REG_STARTIDX].xaddr),
 			MAX_H_REGS,
 			device_path
 		);
@@ -692,7 +692,7 @@ int register_read(modbus_t *mb, int addr, regtype_t type, void *data)
 		 * memory corruptions and buggy inputs below...
 		 */
 		default:
-			upsdebugx(2,"ERROR: register_read: invalid register type %d\n", type);
+			upsdebugx(2, "ERROR: register_read: invalid register type %u", type);
 			break;
 #ifdef __clang__
 # pragma clang diagnostic pop
@@ -703,9 +703,9 @@ int register_read(modbus_t *mb, int addr, regtype_t type, void *data)
 	}
 	if (rval == -1) {
 		upslogx(LOG_ERR,
-			"ERROR:(%s) modbus_read: addr:0x%x, type:%8s, path:%s\n",
+			"ERROR:(%s) modbus_read: addr:0x%x, type:%8s, path:%s",
 			modbus_strerror(errno),
-			addr,
+			(unsigned int)addr,
 			(type == COIL) ? "COIL" :
 			(type == INPUT_B) ? "INPUT_B" :
 			(type == INPUT_R) ? "INPUT_R" : "HOLDING",
@@ -718,7 +718,8 @@ int register_read(modbus_t *mb, int addr, regtype_t type, void *data)
 			modbus_reconnect();
 		}
 	}
-	upsdebugx(3, "register addr: 0x%x, register type: %d read: %u",addr, type, *(unsigned int *)data);
+	upsdebugx(3, "register addr: 0x%x, register type: %u read: %u",
+		(unsigned int)addr, type, *(unsigned int *)data);
 	return rval;
 }
 
@@ -755,14 +756,14 @@ int register_write(modbus_t *mb, int addr, regtype_t type, void *data)
 #if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_COVERED_SWITCH_DEFAULT)
 # pragma GCC diagnostic pop
 #endif
-			upsdebugx(2,"ERROR: register_write: invalid register type %d\n", type);
+			upsdebugx(2, "ERROR: register_write: invalid register type %u", type);
 			break;
 	}
 	if (rval == -1) {
 		upslogx(LOG_ERR,
-			"ERROR:(%s) modbus_write: addr:0x%x, type:%8s, path:%s\n",
+			"ERROR:(%s) modbus_write: addr:0x%x, type:%8s, path:%s",
 			modbus_strerror(errno),
-			addr,
+			(unsigned int)addr,
 			(type == COIL) ? "COIL" :
 			(type == INPUT_B) ? "INPUT_B" :
 			(type == INPUT_R) ? "INPUT_R" : "HOLDING",
@@ -775,7 +776,8 @@ int register_write(modbus_t *mb, int addr, regtype_t type, void *data)
 			modbus_reconnect();
 		}
 	}
-	upsdebugx(3, "register addr: 0x%x, register type: %d read: %u",addr, type, *(unsigned int *)data);
+	upsdebugx(3, "register addr: 0x%x, register type: %u read: %u",
+		(unsigned int)addr, type, *(unsigned int *)data);
 	return rval;
 }
 
@@ -815,16 +817,17 @@ int upscmd(const char *cmd, const char *arg)
 		rval = register_write(mbctx, regs[FSD].xaddr, regs[FSD].type, &data);
 		if (rval == -1) {
 			upslogx(2,
-				"ERROR:(%s) modbus_write_register: addr:0x%08x, regtype: %d, path:%s\n",
+				"ERROR:(%s) modbus_write_register: addr:0x%08x, regtype: %u, path:%s",
 				modbus_strerror(errno),
-				regs[FSD].xaddr,
+				(unsigned int)(regs[FSD].xaddr),
 				regs[FSD].type,
 				device_path
 			);
 			upslogx(LOG_NOTICE, "load.off: failed (communication error) [%s] [%s]", cmd, arg);
 			rval = STAT_INSTCMD_FAILED;
 		} else {
-			upsdebugx(2, "load.off: addr: 0x%x, data: %d", regs[FSD].xaddr, data);
+			upsdebugx(2, "load.off: addr: 0x%x, data: %d",
+				(unsigned int)(regs[FSD].xaddr), data);
 			rval = STAT_INSTCMD_HANDLED;
 		}
 	} else if (!strcasecmp(cmd, "shutdown.stayoff")) {
@@ -843,7 +846,7 @@ int upscmd(const char *cmd, const char *arg)
 
 			/* wait for an increasing time interval before sending shutdown command */
 			while ((etime = time_elapsed(&start)) < ( FSD_REPEAT_INTRV / cnt));
-			upsdebugx(2, "ERROR: load.off failed, wait for %lims, retries left: %d\n", etime, cnt - 1);
+			upsdebugx(2, "ERROR: load.off failed, wait for %lims, retries left: %d", etime, cnt - 1);
 			cnt--;
 		}
 		switch (rval) {
@@ -959,13 +962,13 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
 				char	*reg_val_s;
 
 				state->reg.val.ui16 = reg_val;
-				n = snprintf(NULL, 0, "%d", reg_val);
+				n = snprintf(NULL, 0, "%u", reg_val);
 				if (ptr != NULL) {
 					free(ptr);
 				}
 				reg_val_s = (char *)xmalloc(sizeof(char) * (n + 1));
 				ptr = reg_val_s;
-				sprintf(reg_val_s, "%d", reg_val);
+				sprintf(reg_val_s, "%u", reg_val);
 				state->reg.strval = reg_val_s;
 			} else {
 				state->reg.val.ui16 = 0;
@@ -1203,13 +1206,13 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
 			{ /* scoping */
 				char	*reg_val_s;
 				state->reg.val.ui16 = reg_val;
-				n = snprintf(NULL, 0, "%d", reg_val);
+				n = snprintf(NULL, 0, "%u", reg_val);
 				if (ptr != NULL) {
 					free(ptr);
 				}
 				reg_val_s = (char *)xmalloc(sizeof(char) * (n + 1));
 				ptr = reg_val_s;
-				sprintf(reg_val_s, "%d", reg_val);
+				sprintf(reg_val_s, "%u", reg_val);
 				state->reg.strval = reg_val_s;
 			}
 			break;
@@ -1268,31 +1271,31 @@ void get_config_vars(void)
 	if (testvar("mod_resp_to_s")) {
 		mod_resp_to_s = (uint32_t)strtol(getval("mod_resp_to_s"), NULL, 10);
 	}
-	upsdebugx(2, "mod_resp_to_s %d", mod_resp_to_s);
+	upsdebugx(2, "mod_resp_to_s %u", mod_resp_to_s);
 
 	/* check if response time out (us) is set and get the value */
 	if (testvar("mod_resp_to_us")) {
 		mod_resp_to_us = (uint32_t) strtol(getval("mod_resp_to_us"), NULL, 10);
 		if (mod_resp_to_us > 999999) {
-			fatalx(EXIT_FAILURE, "get_config_vars: Invalid mod_resp_to_us %d", mod_resp_to_us);
+			fatalx(EXIT_FAILURE, "get_config_vars: Invalid mod_resp_to_us %u", mod_resp_to_us);
 		}
 	}
-	upsdebugx(2, "mod_resp_to_us %d", mod_resp_to_us);
+	upsdebugx(2, "mod_resp_to_us %u", mod_resp_to_us);
 
 	/* check if byte time out (s) is set and get the value */
 	if (testvar("mod_byte_to_s")) {
 		mod_byte_to_s = (uint32_t)strtol(getval("mod_byte_to_s"), NULL, 10);
 	}
-	upsdebugx(2, "mod_byte_to_s %d", mod_byte_to_s);
+	upsdebugx(2, "mod_byte_to_s %u", mod_byte_to_s);
 
 	/* check if byte time out (us) is set and get the value */
 	if (testvar("mod_byte_to_us")) {
 		mod_byte_to_us = (uint32_t) strtol(getval("mod_byte_to_us"), NULL, 10);
 		if (mod_byte_to_us > 999999) {
-			fatalx(EXIT_FAILURE, "get_config_vars: Invalid mod_byte_to_us %d", mod_byte_to_us);
+			fatalx(EXIT_FAILURE, "get_config_vars: Invalid mod_byte_to_us %u", mod_byte_to_us);
 		}
 	}
-	upsdebugx(2, "mod_byte_to_us %d", mod_byte_to_us);
+	upsdebugx(2, "mod_byte_to_us %u", mod_byte_to_us);
 }
 
 /* create a new modbus context based on connection type (serial or TCP) */
@@ -1303,21 +1306,21 @@ modbus_t *modbus_new(const char *port)
 	if (strstr(port, "/dev/tty") != NULL) {
 		mb = modbus_new_rtu(port, ser_baud_rate, ser_parity, ser_data_bit, ser_stop_bit);
 		if (mb == NULL) {
-			upslogx(LOG_ERR, "modbus_new_rtu: Unable to open serial port context\n");
+			upslogx(LOG_ERR, "modbus_new_rtu: Unable to open serial port context");
 		}
 	} else if ((sp = strchr(port, ':')) != NULL) {
 		char *tcp_port = xmalloc(sizeof(sp));
 		strcpy(tcp_port, sp + 1);
 		*sp = '\0';
 		mb = modbus_new_tcp(port, (int)strtoul(tcp_port, NULL, 10));
 		if (mb == NULL) {
-			upslogx(LOG_ERR, "modbus_new_tcp: Unable to connect to %s\n", port);
+			upslogx(LOG_ERR, "modbus_new_tcp: Unable to connect to %s", port);
 		}
 		free(tcp_port);
 	} else {
 		mb = modbus_new_tcp(port, 502);
 		if (mb == NULL) {
-			upslogx(LOG_ERR, "modbus_new_tcp: Unable to connect to %s\n", port);
+			upslogx(LOG_ERR, "modbus_new_tcp: Unable to connect to %s", port);
 		}
 	}
 	return mb;