Refactor void* casting to specific types, indentations and other noisy stuff

NEWS.adoc

@@ -89,6 +89,14 @@ https://github.com/networkupstools/nut/milestone/12
      Separated an `upsdrvquery_ping()` operation from earlier code, so it can
      be directly used by consumers (in-tree this is drivers and `upsdrvctl`).
      [issue #3276, PR #3277]
+   * Revised `void*` pointer casting into specific types for values returned
+     by `xmalloc()` and friends, or iterated in loops via `something->next`,
+     as `clang-21` now warns about not-casting void pointers (formally valid,
+     practically uncertain if that code matches the developer's intention),
+     and (better safe than sorry) added `NULL`-return checks although we should
+     not normally get into those code paths with the `x*()` methods which
+     should bail out and abort the program with a common error message when
+     an allocation fails. [#3293]
 
  - NUT for Windows specific updates:
    * Revised detection of (relative) paths to program and configuration files

clients/cgilib.c

@@ -31,7 +31,10 @@ static char *unescape(char *buf)
 	char	ch, *newbuf, hex[8];
 
 	buflen = strlen(buf) + 2;
-	newbuf = xmalloc(buflen);
+	newbuf = (char *)xmalloc(buflen);
+	if (newbuf == NULL) {
+		return NULL;
+	}
 	*newbuf = '\0';
 
 	fflush(stdout);

clients/nutclientmem.cpp

@@ -37,7 +37,10 @@ Device MemClientStub::getDevice(const std::string& name)
 
 std::set<std::string> MemClientStub::getDeviceNames()
 {
-	throw NutException("Not implemented");
+	volatile bool not_implemented = true;
+	if (not_implemented)
+		throw NutException("Not implemented");
+	return std::set<std::string>();
 }
 
 std::string MemClientStub::getDeviceDescription(const std::string& name)
@@ -176,7 +179,10 @@ TrackingID MemClientStub::executeDeviceCommand(const std::string& dev, const std
 
 std::map<std::string, std::set<std::string>> MemClientStub::listDeviceClients(void)
 {
-	throw NutException("Not implemented");
+	volatile bool not_implemented = true;
+	if (not_implemented)
+		throw NutException("Not implemented");
+	return std::map<std::string, std::set<std::string>>();
 }
 
 std::set<std::string> MemClientStub::deviceGetClients(const std::string& dev)

clients/upsc.c

@@ -87,7 +87,7 @@ static void usage(const char *prog)
 	printf("\nCommon arguments:\n");
 	printf("  -V         - display the version of this software\n");
 	printf("  -W <secs>  - network timeout for initial connections (default: %s)\n",
-	       UPSCLI_DEFAULT_CONNECT_TIMEOUT);
+		UPSCLI_DEFAULT_CONNECT_TIMEOUT);
 	printf("  -h         - display this help text\n");
 
 	nut_report_config_flags();
@@ -463,7 +463,7 @@ int main(int argc, char **argv)
 	upsdebugx(1, "upsname='%s' hostname='%s' port='%" PRIu16 "'",
 		NUT_STRARG(upsname), NUT_STRARG(hostname), port);
 
-	ups = xmalloc(sizeof(*ups));
+	ups = (UPSCONN_t *)xmalloc(sizeof(*ups));
 
 	if (upscli_connect(ups, hostname, port, UPSCLI_CONN_TRYSSL) < 0) {
 		fatalx_error_json_simple(0, upscli_strerror(ups));

clients/upsclient.c

@@ -1972,7 +1972,7 @@ int upscli_init_default_connect_timeout(const char *cli_secs, const char *config
 	}
 
 	upsdebugx(1, "%s: upscli_default_connect_timeout=%" PRIiMAX
-		 ".%06" PRIiMAX " sec assigned from: %s",
+		".%06" PRIiMAX " sec assigned from: %s",
 		__func__, (intmax_t)upscli_default_connect_timeout.tv_sec,
 		(intmax_t)upscli_default_connect_timeout.tv_usec, cause);
 

clients/upscmd.c

@@ -70,7 +70,7 @@ static void usage(const char *prog)
 	printf("\nCommon arguments:\n");
 	printf("  -V         - display the version of this software\n");
 	printf("  -W <secs>  - network timeout for initial connections (default: %s)\n",
-	       UPSCLI_DEFAULT_CONNECT_TIMEOUT);
+		UPSCLI_DEFAULT_CONNECT_TIMEOUT);
 	printf("  -h         - display this help text\n");
 
 	nut_report_config_flags();
@@ -135,7 +135,7 @@ static void listcmds(void)
 		/* we must first read the entire list of commands,
 		 * before we can start reading the descriptions */
 
-		ltmp = xcalloc(1, sizeof(*ltmp));
+		ltmp = (struct list_t *)xcalloc(1, sizeof(*ltmp));
 		ltmp->name = xstrdup(answer[2]);
 
 		if (llast) {
@@ -377,7 +377,7 @@ int main(int argc, char **argv)
 		fatalx(EXIT_FAILURE, "Error: invalid UPS definition.  Required format: upsname[@hostname[:port]]");
 	}
 
-	ups = xcalloc(1, sizeof(*ups));
+	ups = (UPSCONN_t *)xcalloc(1, sizeof(*ups));
 
 	if (upscli_connect(ups, hostname, port, UPSCLI_CONN_TRYSSL) < 0) {
 		fatalx(EXIT_FAILURE, "Error: %s", upscli_strerror(ups));

clients/upsimagearg.h

@@ -59,8 +59,8 @@ static struct {
 
 typedef struct {
 	char	*name;		/* name of the UPS variable                 */
-	char	*minimum;	/* name of minimum value UPS variable
-	                       or variable in imgarg table              */
+	char	*minimum;	/* name of minimum value UPS variable       */
+				/* or variable in imgarg table              */
 	char	*nominal;	/* as above, only for nominal value         */
 	char	*maximum;	/* as above, only for maximum value         */
 	int	deviation;	/* variable deviation - width of green zone */

clients/upslog.c

@@ -99,7 +99,7 @@ static struct logtarget_t *add_logfile(const char *logfn_arg)
 	if (!logfn_arg || !(*logfn_arg))
 		return p;
 
-	p = xcalloc(1, sizeof(struct monhost_ups_t));
+	p = (struct logtarget_t *)xcalloc(1, sizeof(struct logtarget_t));
 	p->logfn = xstrdup(logfn_arg);
 	p->logfile = NULL;
 
@@ -129,8 +129,8 @@ static void reopen_log(void)
 		}
 
 		if ((p->logfile = freopen(
-		    p->logfn, "a",
-		    p->logfile)) == NULL
+			p->logfn, "a",
+			p->logfile)) == NULL
 		) {
 			fatal_with_errno(EXIT_FAILURE,
 				"could not reopen logfile %s", p->logfn);
@@ -225,7 +225,7 @@ static void help(const char *prog)
 	printf("\nCommon arguments:\n");
 	printf("  -V         - display the version of this software\n");
 	printf("  -W <secs>  - network timeout for initial connections (default: %s)\n",
-	       UPSCLI_DEFAULT_CONNECT_TIMEOUT);
+		UPSCLI_DEFAULT_CONNECT_TIMEOUT);
 	printf("  -h         - display this help text\n");
 	printf("\n");
 	printf("Some valid format string escapes:\n");
@@ -374,7 +374,7 @@ static void add_call(void (*fptr)(const char *arg, const struct monhost_ups_t *m
 		tmp = tmp->next;
 	}
 
-	tmp = xmalloc(sizeof(flist_t));
+	tmp = (flist_t *)xmalloc(sizeof(flist_t));
 
 	tmp->fptr = fptr;
 
@@ -543,7 +543,7 @@ int main(int argc, char **argv)
 					char *m_arg, *s;
 
 					monhost_ups_prev = monhost_ups_current;
-					monhost_ups_current = xmalloc(sizeof(struct monhost_ups_t));
+					monhost_ups_current = (struct monhost_ups_t *)xmalloc(sizeof(struct monhost_ups_t));
 					if (monhost_ups_anchor == NULL)
 						monhost_ups_anchor = monhost_ups_current;
 					else
@@ -685,7 +685,7 @@ int main(int argc, char **argv)
 	if (argc >= 4) {
 		/* read out the remaining argv entries to the format string */
 
-		logformat = xmalloc(LARGEBUF);
+		logformat = (char *)xmalloc(LARGEBUF);
 		memset(logformat, '\0', LARGEBUF);
 		logformat_allocated = 1;
 
@@ -703,7 +703,7 @@ int main(int argc, char **argv)
 
 		/* May be or not be NULL here: */
 		monhost_ups_prev = monhost_ups_current;
-		monhost_ups_current = xmalloc(sizeof(struct monhost_ups_t));
+		monhost_ups_current = (struct monhost_ups_t *)xmalloc(sizeof(struct monhost_ups_t));
 		if (monhost_ups_anchor == NULL) {
 			/* Become the single-entry list */
 			monhost_ups_anchor = monhost_ups_current;
@@ -731,7 +731,7 @@ int main(int argc, char **argv)
 		char	*s = xstrdup(logformat);
 		if (s) {
 			if (!logformat_allocated) {
-				logformat = xmalloc(LARGEBUF);
+				logformat = (char *)xmalloc(LARGEBUF);
 				if (!logformat)
 					fatalx(EXIT_FAILURE, "Failed re-allocation to prepend UPSHOST to formatting string");
 				memset(logformat, '\0', LARGEBUF);
@@ -783,7 +783,7 @@ int main(int argc, char **argv)
 				monhost_ups_current->port
 			);
 
-			conn = xmalloc(sizeof(*conn));
+			conn = (UPSCONN_t *)xmalloc(sizeof(*conn));
 
 			if (upscli_connect(conn, monhost_ups_current->hostname, monhost_ups_current->port, UPSCLI_CONN_TRYSSL) < 0) {
 				fatalx(EXIT_FAILURE, "Error: %s", upscli_strerror(conn));
@@ -811,7 +811,13 @@ int main(int argc, char **argv)
 				found++;
 				upsdebugx(1, "FOUND: %s: %s", answer[1], answer[2]);
 
-				mu = xmalloc(sizeof(struct monhost_ups_t));
+				mu = (struct monhost_ups_t *)xmalloc(sizeof(struct monhost_ups_t));
+				if (mu == NULL) {
+					upslogx(LOG_ERR, "Failed to get memory for monitoring host structure. Not adding %s@%s:%" PRIu16,
+						answer[1], monhost_ups_current->hostname, monhost_ups_current->port);
+					continue;
+				}
+
 				snprintf(buf, sizeof(buf), "%s@%s:%" PRIu16,
 					answer[1],
 					monhost_ups_current->hostname,
@@ -897,7 +903,7 @@ int main(int argc, char **argv)
 			fatalx(EXIT_FAILURE, "Error: invalid UPS definition.  Required format: upsname[@hostname[:port]]\n");
 		}
 
-		monhost_ups_current->ups = xmalloc(sizeof(UPSCONN_t));
+		monhost_ups_current->ups = (UPSCONN_t *)xmalloc(sizeof(UPSCONN_t));
 
 		if (upscli_connect(monhost_ups_current->ups, monhost_ups_current->hostname, monhost_ups_current->port, UPSCLI_CONN_TRYSSL) < 0)
 			fprintf(stderr, "Warning: initial connect failed: %s\n",

clients/upsmon.c

@@ -1104,7 +1104,7 @@ static void doshutdown(void)
 
 			/* Contact the data server(s) regularly so this
 			 * client is not assumed dead while looping */
-			for (ups = firstups; ups != NULL && !exit_flag; ups = ups->next) {
+			for (ups = firstups; ups != NULL && !exit_flag; ups = (utype_t *)ups->next) {
 				set_alarm();
 
 				if (get_var(ups, "numlogins", temp, sizeof(temp)) >= 0) {
@@ -1337,7 +1337,7 @@ static void sync_secondaries(void)
 
 		upsnotify(NOTIFY_STATE_WATCHDOG, NULL);
 
-		for (ups = firstups; ups != NULL; ups = ups->next) {
+		for (ups = firstups; ups != NULL; ups = (utype_t *)ups->next) {
 
 			/* only check login count on devices we are the primary for */
 			if (!flag_isset(ups->status, ST_PRIMARY))
@@ -1404,7 +1404,7 @@ static void forceshutdown(void)
 		userfsd, exit_flag, use_pipe);
 
 	/* set FSD on any "primary" UPS entries (forced shutdown in progress) */
-	for (ups = firstups; ups != NULL; ups = ups->next)
+	for (ups = firstups; ups != NULL; ups = (utype_t *)ups->next)
 		if (flag_isset(ups->status, ST_PRIMARY)) {
 			isaprimary = 1;
 			upsdebugx(2, "%s: tell data server to setfsd(%s@%s)",
@@ -1594,7 +1594,7 @@ static int is_ups_critical(utype_t *ups)
 	if (flag_isset(ups->status, ST_CAL)) {
 		upslogx(LOG_WARNING, "%s: seems that UPS [%s] is OB+LB now, but "
 			"it is also calibrating - not declaring a critical state",
-			  __func__, ups->upsname);
+			__func__, ups->upsname);
 		return 0;
 	}
 
@@ -1723,7 +1723,7 @@ static void recalc(void)
 		else
 			val_ol += ups->pv;
 
-		ups = ups->next;
+		ups = (utype_t *)ups->next;
 	}
 
 	upsdebugx(3, "Current power value: %u", val_ol);
@@ -2130,10 +2130,10 @@ static void addups(int reloading, const char *sys, const char *pvs,
 			return;
 		}
 
-		tmp = tmp->next;
+		tmp = (utype_t *)tmp->next;
 	}
 
-	tmp = xcalloc(1, sizeof(utype_t));
+	tmp = (utype_t *)xcalloc(1, sizeof(utype_t));
 	/* TOTHINK: init (UPSCONN_t)tmp->conn struct fields too? */
 	tmp->sys = xstrdup(sys);
 	tmp->pv = pv;
@@ -2620,7 +2620,7 @@ static void loadconfig(void)
 			while (ups) {
 				ups->pollfail_log_throttle_count = -1;
 				ups->pollfail_log_throttle_state = UPSCLI_ERR_NONE;
-				ups = ups->next;
+				ups = (utype_t *)ups->next;
 			}
 		}
 	}
@@ -2740,7 +2740,7 @@ static void upsmon_cleanup(void)
 	utmp = firstups;
 
 	while (utmp) {
-		unext = utmp->next;
+		unext = (utype_t *)utmp->next;
 
 		drop_connection(utmp);
 		ups_free(utmp);
@@ -3442,7 +3442,7 @@ static void help(const char *arg_progname)
 	printf("\nCommon arguments:\n");
 	printf("  -V         - display the version of this software\n");
 	printf("  -W <secs>  - network timeout for initial connections (default: %s)\n",
-	       UPSCLI_DEFAULT_CONNECT_TIMEOUT);
+		UPSCLI_DEFAULT_CONNECT_TIMEOUT);
 	printf("  -h         - display this help text\n");
 
 	nut_report_config_flags();
@@ -3625,7 +3625,7 @@ static void delete_ups(utype_t *target)
 
 			/* about to delete the first ups? */
 			if (ptr == last)
-				firstups = ptr->next;
+				firstups = (utype_t *)ptr->next;
 			else
 				last->next = ptr->next;
 
@@ -3637,7 +3637,7 @@ static void delete_ups(utype_t *target)
 		}
 
 		last = ptr;
-		ptr = ptr->next;
+		ptr = (utype_t *)ptr->next;
 	}
 
 	/* shouldn't happen */
@@ -3677,7 +3677,7 @@ static void reload_conf(void)
 
 	while (tmp) {
 		tmp->retain = 0;
-		tmp = tmp->next;
+		tmp = (utype_t *)tmp->next;
 	}
 
 	/* reset paranoia checker */
@@ -3688,9 +3688,8 @@ static void reload_conf(void)
 
 	/* go through the utype_t struct again */
 	tmp = firstups;
-
 	while (tmp) {
-		next = tmp->next;
+		next = (utype_t *)tmp->next;
 
 		/* !retain means it wasn't in the .conf this time around */
 		if (tmp->retain == 0)
@@ -3816,9 +3815,9 @@ int main(int argc, char *argv[])
 	print_banner_once(prog, 0);
 
 	/* if no configuration file is specified on the command line, use default */
-	configfile = xmalloc(SMALLBUF);
+	configfile = (char *)xmalloc(SMALLBUF);
 	snprintf(configfile, SMALLBUF, "%s/upsmon.conf", confpath());
-	configfile = xrealloc(configfile, strlen(configfile) + 1);
+	configfile = (char *)xrealloc(configfile, strlen(configfile) + 1);
 
 	run_as_user = xstrdup(RUN_AS_USER);
 
@@ -4258,7 +4257,7 @@ int main(int argc, char *argv[])
 
 				gettimeofday(&now, NULL);
 				time(&ttNow);
-				for (ups = firstups; ups != NULL; ups = ups->next) {
+				for (ups = firstups; ups != NULL; ups = (utype_t *)ups->next) {
 					ups->status = 0;
 					ups->lastpoll = ttNow;
 				}
@@ -4280,7 +4279,7 @@ int main(int argc, char *argv[])
 		/* Reset the value, regardless of support */
 		sleep_inhibitor_status = -2;
 
-		for (ups = firstups; ups != NULL; ups = ups->next) {
+		for (ups = firstups; ups != NULL; ups = (utype_t *)ups->next) {
 			if (isPreparingForSleepSupported() && (sleep_inhibitor_status = isPreparingForSleep()) >= 0) {
 				upsdebugx(2, "Aborting UPS polling sub-loop because OS is preparing for sleep or just woke up");
 				goto end_loop_cycle;