Fightwarn: numerous complaints from clang-15 [#823]

Surprised that so many of these were not found sooner.
Almost all fall into a few simple categories:
* method(void) declared without a "void"
* declarations after code
** NUT_UNUSED_VARIABLE() is code
** scoping in switch() cases
* a few smartly detected unused variables => added reasonable use-cases (logging or better)

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

Author: jimklimov

NEWS.adoc

@@ -113,6 +113,8 @@ as part of https://github.com/networkupstools/nut/issues/1410 solution.
    * Something about compile-time macros or other warnings-related refactoring
      seems to have confused the MGE SHUT (Serial HID UPS Transfer) driver
      support [#2022]
+   * Some warnings were not detected by the tools or build scenarios used
+     earlier, and only got addressed now
 
  - An issue was identified which could cause `libupsclient` parser of device
    and host names to crash upon bad inputs (e.g. poorly resolved environment

clients/cgilib.c

@@ -40,6 +40,7 @@ static char *unescape(char *buf)
 			ch = ' ';
 
 		if (ch == '%') {
+			long l;
 			if (i + 2 > buflen)
 				fatalx(EXIT_FAILURE, "string too short for escaped char");
 			hex[0] = buf[++i];
@@ -48,7 +49,7 @@ static char *unescape(char *buf)
 			if (!isxdigit((unsigned char) hex[0])
 				|| !isxdigit((unsigned char) hex[1]))
 				fatalx(EXIT_FAILURE, "bad escape char");
-			long l = strtol(hex, NULL, 16);
+			l = strtol(hex, NULL, 16);
 			assert(l>=0);
 			assert(l<=255);
 			ch = (char)l;	/* FIXME: Loophole about non-ASCII symbols in top 128 values, or negatives for signed char... */

clients/upsclient.c

@@ -329,6 +329,7 @@ static void HandshakeCallback(PRFileDesc *fd, UPSCONN_t *client_data)
 int upscli_init(int certverify, const char *certpath,
 					const char *certname, const char *certpasswd)
 {
+	const char *quiet_init_ssl;
 #ifdef WITH_OPENSSL
 	long ret;
 	int ssl_mode = SSL_VERIFY_NONE;
@@ -343,7 +344,7 @@ int upscli_init(int certverify, const char *certpath,
 	NUT_UNUSED_VARIABLE(certpasswd);
 #endif /* WITH_OPENSSL | WITH_NSS */
 
-	const char *quiet_init_ssl = getenv("NUT_QUIET_INIT_SSL");
+	quiet_init_ssl = getenv("NUT_QUIET_INIT_SSL");
 	if (quiet_init_ssl != NULL) {
 		if (*quiet_init_ssl == '\0'
 			|| (strncmp(quiet_init_ssl, "true", 4)
@@ -678,8 +679,9 @@ static ssize_t net_read(UPSCONN_t *ups, char *buf, size_t buflen, const time_t t
 		 * 32-bit builds)... Not likely to exceed in 64-bit builds,
 		 * but smaller systems with 16-bits might be endangered :)
 		 */
+		int iret;
 		assert(buflen <= INT_MAX);
-		int iret = SSL_read(ups->ssl, buf, (int)buflen);
+		iret = SSL_read(ups->ssl, buf, (int)buflen);
 		assert(iret <= SSIZE_MAX);
 		ret = (ssize_t)iret;
 #elif defined(WITH_NSS) /* WITH_OPENSSL */
@@ -762,8 +764,9 @@ static ssize_t net_write(UPSCONN_t *ups, const char *buf, size_t buflen, const t
 		 * 32-bit builds)... Not likely to exceed in 64-bit builds,
 		 * but smaller systems with 16-bits might be endangered :)
 		 */
+		int iret;
 		assert(buflen <= INT_MAX);
-		int iret = SSL_write(ups->ssl, buf, (int)buflen);
+		iret = SSL_write(ups->ssl, buf, (int)buflen);
 		assert(iret <= SSIZE_MAX);
 		ret = (ssize_t)iret;
 #elif defined(WITH_NSS) /* WITH_OPENSSL */

clients/upslog.c

@@ -559,9 +559,9 @@ int main(int argc, char **argv)
 			monhost_ups_anchor = monhost_ups_current;
 			monhost_ups_current->next = NULL;
 			monhost_ups_current->monhost = monhost;
-			monhost_len=1;
+			monhost_len = 1;
 		} else {
-			fatalx(EXIT_FAILURE, "No UPS defined for monitoring - use -s <system>");
+			fatalx(EXIT_FAILURE, "No UPS defined for monitoring - use -s <system> or -m <ups,logfile>");
 		}
 
 		if (logfn)
@@ -574,6 +574,10 @@ int main(int argc, char **argv)
 	if (!logformat)
 		fatalx(EXIT_FAILURE, "No format defined - but this should be impossible");
 
+	/* shouldn't happen */
+	if (!monhost_len)
+		fatalx(EXIT_FAILURE, "No UPS defined for monitoring - use -s <system> or -m <ups,logfile>");
+
 	for (monhost_ups_current = monhost_ups_anchor;
 	     monhost_ups_current != NULL;
 	     monhost_ups_current = monhost_ups_current->next) {

clients/upsmon.c

@@ -1182,6 +1182,7 @@ static void addups(int reloading, const char *sys, const char *pvs,
 {
 	unsigned int	pv;
 	utype_t	*tmp, *last;
+	long	lpv;
 
 	/* the username is now required - no more host-based auth */
 
@@ -1191,7 +1192,7 @@ static void addups(int reloading, const char *sys, const char *pvs,
 		return;
 	}
 
-	long lpv = strtol(pvs, (char **) NULL, 10);
+	lpv = strtol(pvs, (char **) NULL, 10);
 
 #if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && ( (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TYPE_LIMITS) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE) || (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE) )
 # pragma GCC diagnostic push

clients/upsset.c

@@ -751,11 +751,12 @@ static void do_type(const char *varname)
 
 		if (!strncasecmp(answer[i], "STRING:", 7)) {
 			char	*ptr, len;
+			long	l;
 
 			/* split out the :<len> data */
 			ptr = strchr(answer[i], ':');
 			*ptr++ = '\0';
-			long l = strtol(ptr, (char **) NULL, 10);
+			l = strtol(ptr, (char **) NULL, 10);
 			assert(l <= 127);	/* FIXME: Loophole about longer numbers? Why are we limited to char at all here? */
 			len = (char)l;
 

common/common.c

@@ -78,7 +78,7 @@ const char *UPS_VERSION = NUT_VERSION_MACRO;
 #include <sys/types.h>
 #include <limits.h>
 #include <stdlib.h>
-pid_t get_max_pid_t()
+pid_t get_max_pid_t(void)
 {
 #ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE
 #pragma GCC diagnostic push
@@ -1948,9 +1948,10 @@ static char * get_libname_in_dir(const char* base_libname, size_t base_libname_l
 #if !HAVE_DECL_REALPATH
 		struct stat	st;
 #endif
+		int compres;
 
 		upsdebugx(5,"Comparing lib %s with dirpath entry %s", base_libname, dirp->d_name);
-		int compres = strncmp(dirp->d_name, base_libname, base_libname_length);
+		compres = strncmp(dirp->d_name, base_libname, base_libname_length);
 		if (compres == 0
 		&&  dirp->d_name[base_libname_length] == '\0' /* avoid "*.dll.a" etc. */
 		) {

drivers/adelsystem_cbi.c

@@ -900,13 +900,16 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
 		case LVDC:					/* "output.voltage" */
 		case LCUR:					/* "output.current" */
 			if (reg_val != 0) {
+				char	*fval_s;
+				double	fval;
+
 				state->reg.val.ui16 = reg_val;
-				double fval = reg_val / 1000.00; /* convert mV to V, mA to A */
+				fval = reg_val / 1000.00; /* convert mV to V, mA to A */
 				n = snprintf(NULL, 0, "%.2f", fval);
 				if (ptr != NULL) {
 					free(ptr);
 				}
-				char *fval_s = (char *)xmalloc(sizeof(char) * (n + 1));
+				fval_s = (char *)xmalloc(sizeof(char) * (n + 1));
 				ptr = fval_s;
 				sprintf(fval_s, "%.2f", fval);
 				state->reg.strval = fval_s;
@@ -921,12 +924,14 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
 		case BCEF:
 		case VAC:					/* "input.voltage" */
 			if (reg_val != 0) {
+				char	*reg_val_s;
+
 				state->reg.val.ui16 = reg_val;
 				n = snprintf(NULL, 0, "%d", reg_val);
 				if (ptr != NULL) {
 					free(ptr);
 				}
-				char *reg_val_s = (char *)xmalloc(sizeof(char) * (n + 1));
+				reg_val_s = (char *)xmalloc(sizeof(char) * (n + 1));
 				ptr = reg_val_s;
 				sprintf(reg_val_s, "%d", reg_val);
 				state->reg.strval = reg_val_s;
@@ -938,13 +943,16 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
 			break;
 		case BSOC:					/* "battery.charge" */
 			if (reg_val != 0) {
+				double	fval;
+				char	*fval_s;
+
 				state->reg.val.ui16 = reg_val;
-				double fval = (double )reg_val * regs[BSOC].scale;
+				fval = (double )reg_val * regs[BSOC].scale;
 				n = snprintf(NULL, 0, "%.2f", fval);
 				if (ptr != NULL) {
 					free(ptr);
 				}
-				char *fval_s = (char *)xmalloc(sizeof(char) * (n + 1));
+				fval_s = (char *)xmalloc(sizeof(char) * (n + 1));
 				ptr = fval_s;
 				sprintf(fval_s, "%.2f", fval);
 				state->reg.strval = fval_s;
@@ -956,16 +964,21 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
 			break;
 		case BTMP:					/* "battery.temperature" */
 		case OTMP:					/* "ups.temperature" */
-			state->reg.val.ui16 = reg_val;
-			double fval = reg_val - 273.15;
-			n = snprintf(NULL, 0, "%.2f", fval);
-			char *fval_s = (char *)xmalloc(sizeof(char) * (n + 1));
-			if (ptr != NULL) {
-				free(ptr);
+			{ /* scoping */
+				double	fval;
+				char	*fval_s;
+
+				state->reg.val.ui16 = reg_val;
+				fval = reg_val - 273.15;
+				n = snprintf(NULL, 0, "%.2f", fval);
+				fval_s = (char *)xmalloc(sizeof(char) * (n + 1));
+				if (ptr != NULL) {
+					free(ptr);
+				}
+				ptr = fval_s;
+				sprintf(fval_s, "%.2f", fval);
+				state->reg.strval = fval_s;
 			}
-			ptr = fval_s;
-			sprintf(fval_s, "%.2f", fval);
-			state->reg.strval = fval_s;
 			upsdebugx(3, "get_dev_state: variable: %s", state->reg.strval);
 			break;
 		case PMNG:					/* "ups.status" & "battery.charge" */
@@ -1155,15 +1168,18 @@ int get_dev_state(devreg_t regindx, devstate_t **dvstat)
 		 * memory corruptions and buggy inputs below...
 		 */
 		default:
-			state->reg.val.ui16 = reg_val;
-			n = snprintf(NULL, 0, "%d", reg_val);
-			if (ptr != NULL) {
-				free(ptr);
+			{ /* scoping */
+				char	*reg_val_s;
+				state->reg.val.ui16 = reg_val;
+				n = snprintf(NULL, 0, "%d", 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);
+				state->reg.strval = reg_val_s;
 			}
-			char *reg_val_s = (char *)xmalloc(sizeof(char) * (n + 1));
-			ptr = reg_val_s;
-			sprintf(reg_val_s, "%d", reg_val);
-			state->reg.strval = reg_val_s;
 			break;
 #ifdef __clang__
 # pragma clang diagnostic pop

drivers/al175.c

@@ -553,9 +553,9 @@ static int al_parse_reply(io_head_t *io_head, raw_data_t *io_buf, /*const*/ raw_
  *     begin                                                                        end
  */
 
-	int err;
-	size_t i;
-	const byte_t *reply = NULL;
+	int	err;
+	size_t	i, io_buf_len;
+	const byte_t	*reply = NULL;
 
 	/* 1: extract header and parse it */
 	/*const*/ raw_data_t raw_reply_head = raw_reply;
@@ -578,7 +578,7 @@ static int al_parse_reply(io_head_t *io_head, raw_data_t *io_buf, /*const*/ raw_
 	}
 
 
-	/* extract the data */
+	/* 3: extract the data */
 	if (io_buf->buf_size < io_head->len)	{
 		upsdebugx(3, "%s: too much data to fit in io_buf\t(%" PRIuSIZE " > %" PRIuSIZE ")",
 				__func__, io_head->len, io_buf->buf_size);
@@ -592,7 +592,7 @@ static int al_parse_reply(io_head_t *io_head, raw_data_t *io_buf, /*const*/ raw_
 		*(io_buf->end++) = reply[11+i];
 
 	assert(io_buf->end - io_buf->begin >= 0);
-	size_t io_buf_len = (size_t)(io_buf->end - io_buf->begin);
+	io_buf_len = (size_t)(io_buf->end - io_buf->begin);
 	reverse_bits(io_buf->begin, io_buf_len );
 
 	upsdebug_hex(3, "\t\t--> payload", io_buf->begin, io_buf_len);
@@ -670,14 +670,15 @@ static int al_check_ack(/*const*/ raw_data_t raw_ack)
 /* clear any flow control (copy from powercom.c) */
 static void ser_disable_flow_control (void)
 {
-	struct termios tio;
+	struct termios	tio;
+	tcflag_t	x;
 
 	tcgetattr (upsfd, &tio);
 
 	/* Clumsy rewrite of a one-liner
 	 *   tio.c_iflag &= ~ (IXON | IXOFF);
 	 * to avoid type conversion warnings */
-	tcflag_t x = (IXON | IXOFF);
+	x = (IXON | IXOFF);
 	tio.c_iflag &= ~ x;
 	tio.c_cc[VSTART] = _POSIX_VDISABLE;
 	tio.c_cc[VSTOP] = _POSIX_VDISABLE;
@@ -688,7 +689,7 @@ static void ser_disable_flow_control (void)
 	tcsetattr(upsfd, TCSANOW, &tio);
 }
 
-static void flush_rx_queue()
+static void flush_rx_queue(void)
 {
 	ser_flush_in(upsfd, "", /*verbose=*/nut_debug_level);
 }
@@ -702,10 +703,11 @@ static void flush_rx_queue()
  */
 static int tx(const char *dmsg, /*const*/ raw_data_t frame)
 {
-	ssize_t err;
+	ssize_t	err;
+	size_t	frame_len;
 
 	assert(frame.end - frame.begin >= 0);
-	size_t frame_len = (size_t)(frame.end - frame.begin);
+	frame_len = (size_t)(frame.end - frame.begin);
 
 	upsdebug_ascii(3, dmsg, frame.begin, frame_len);
 
@@ -807,9 +809,9 @@ static int scan_for(char c)
  *
  * @return 0 (ok) -1 (error)
  */
-static int recv_command_ack()
+static int recv_command_ack(void)
 {
-	ssize_t err;
+	ssize_t	err;
 	raw_data_t ack;
 	byte_t     ack_buf[8];
 
@@ -855,12 +857,12 @@ static int recv_command_ack()
  */
 static int recv_register_data(io_head_t *io, raw_data_t *io_buf)
 {
-	ssize_t err;
-	int ret;
-	raw_data_t reply_head;
-	raw_data_t reply;
-
-	byte_t     reply_head_buf[11];
+	ssize_t	err;
+	int	ret;
+	size_t	reply_head_len;
+	raw_data_t	reply_head;
+	raw_data_t	reply;
+	byte_t	reply_head_buf[11];
 
 	/* 1:  STX  */
 	err = scan_for(STX);
@@ -897,7 +899,7 @@ static int recv_register_data(io_head_t *io, raw_data_t *io_buf)
 	reply = raw_xmalloc(11/*head*/ + io->len/*data*/ + 2/*ETX BCC*/);
 
 	assert (reply_head.end - reply_head.begin >= 0);
-	size_t reply_head_len = (size_t)(reply_head.end - reply_head.begin);
+	reply_head_len = (size_t)(reply_head.end - reply_head.begin);
 
 	memcpy(reply.end, reply_head.begin, reply_head_len);
 	reply.end += reply_head_len;
@@ -1053,9 +1055,9 @@ ACT	SWITCH_TEMP_COMP	(uint16_t on);
 ACT	SWITCH_SYM_ALARM	(void);
 
 /* Implement */
-ACT	TOGGLE_PRS_ONOFF	()		{ return al175_do(0x81, 0x80			Z3);	}
-ACT	CANCEL_BOOST		()		{ return al175_do(0x82, 0x80			Z3);	}
-ACT	STOP_BATTERY_TEST	()		{ return al175_do(0x83, 0x80			Z3);	}
+ACT	TOGGLE_PRS_ONOFF	(void)		{ return al175_do(0x81, 0x80			Z3);	}
+ACT	CANCEL_BOOST		(void)		{ return al175_do(0x82, 0x80			Z3);	}
+ACT	STOP_BATTERY_TEST	(void)		{ return al175_do(0x83, 0x80			Z3);	}
 ACT	START_BATTERY_TEST	(VV_t EndVolt, mm_t Minutes)
 						{ return al175_do(0x83, 0x81, EndVolt, Minutes	Z1);	}
 
@@ -1068,8 +1070,8 @@ ACT	SET_DISCONNECT_LEVEL_AND_DELAY
 				(VV_t level, mm_t delay)
 						{ return al175_do(0x87, 0x84, level, delay	Z1);	}
 
-ACT	RESET_ALARMS		()		{ return al175_do(0x88, 0x80			Z3);	}
-ACT	CHANGE_COMM_PROTOCOL	()		{ return al175_do(0x89, 0x80			Z3);	}
+ACT	RESET_ALARMS		(void)		{ return al175_do(0x88, 0x80			Z3);	}
+ACT	CHANGE_COMM_PROTOCOL	(void)		{ return al175_do(0x89, 0x80			Z3);	}
 ACT	SET_VOLTAGE_AT_ZERO_T	(VV_t v)	{ return al175_do(0x8a, 0x80, v			Z2);	}
 ACT	SET_SLOPE_AT_ZERO_T	(VV_t mv_per_degree)
 						{ return al175_do(0x8a, 0x81, mv_per_degree	Z2);	}
@@ -1078,7 +1080,7 @@ ACT	SET_MAX_TCOMP_VOLTAGE	(VV_t v)	{ return al175_do(0x8a, 0x82, v			Z2);	}
 ACT	SET_MIN_TCOMP_VOLTAGE	(VV_t v)	{ return al175_do(0x8a, 0x83, v			Z2);	}
 ACT	SWITCH_TEMP_COMP	(uint16_t on)	{ return al175_do(0x8b, 0x80, on		Z2);	}
 
-ACT	SWITCH_SYM_ALARM	()		{ return al175_do(0x8c, 0x80			Z3);	}
+ACT	SWITCH_SYM_ALARM	(void)		{ return al175_do(0x8c, 0x80			Z3);	}
 
 
 /**