Merge pull request #2583 from jimklimov/issue-2573

Fix uses of `banner_is_disabled()` to not preclude `-V` printouts; common formation of the banner string; better detection of `time.h` methods

NEWS.adoc

@@ -60,6 +60,8 @@ https://github.com/networkupstools/nut/milestone/11
      sockets and a tight polling loop that hogged CPU. While the underlying
      bug is ancient, it took recent development to hit it as a practical
      regression. [issue #1904, issue #2484]
+   * Fallback `localtime_r()` and `gmtime_r()` for some platform builds where
+     a `*_s()` variant was available was not handled correctly. [PR #2583]
 
  - development iterations of NUT should now identify with not only the semantic
    version of a preceding release, but with git-derived information about the
@@ -93,7 +95,7 @@ https://github.com/networkupstools/nut/milestone/11
 
  - A new `NUT_QUIET_INIT_BANNER` envvar (presence or "true" value) can now
    prevent the tool name and NUT version banner from being unilaterally
-   printed out when NUT programs start. [issues #1789 vs. #316]
+   printed out when NUT programs start. [issues #1789 vs. #316; #2573]
 
  - Extended `upsdrvctl` with a `list` operation (or `-l` option) to report
    manageable device configuration names (possible `<ups>` arguments to

clients/upsc.c

@@ -36,15 +36,14 @@ static UPSCONN_t	*ups = NULL;
 
 static void usage(const char *prog)
 {
-	printf("Network UPS Tools upsc %s\n\n", UPS_VERSION);
+	print_banner_once(prog, 2);
+	printf("NUT read-only client program to display UPS variables.\n");
 
-	printf("usage: %s -l | -L [<hostname>[:port]]\n", prog);
+	printf("\nusage: %s -l | -L [<hostname>[:port]]\n", prog);
 	printf("       %s <ups> [<variable>]\n", prog);
 	printf("       %s -c <ups>\n", prog);
 
-	printf("\nDemo program to display UPS variables.\n\n");
-
-	printf("First form (lists UPSes):\n");
+	printf("\nFirst form (lists UPSes):\n");
 	printf("  -l         - lists each UPS on <hostname>, one per line.\n");
 	printf("  -L         - lists each UPS followed by its description (from ups.conf).\n");
 	printf("               Default hostname: localhost\n");
@@ -251,12 +250,11 @@ int main(int argc, char **argv)
 			break;
 
 		case 'V':
+			/* just show the version and optional
+			 * CONFIG_FLAGS banner if available */
+			print_banner_once(prog, 1);
 			nut_report_config_flags();
-
-			fatalx(EXIT_SUCCESS, "Network UPS Tools upsc %s", UPS_VERSION);
-#ifndef HAVE___ATTRIBUTE__NORETURN
-			exit(EXIT_SUCCESS);	/* Should not get here in practice, but compiler is afraid we can fall through */
-#endif
+			exit(EXIT_SUCCESS);
 
 		case 'h':
 		default:

clients/upscmd.c

@@ -47,11 +47,12 @@ struct list_t {
 
 static void usage(const char *prog)
 {
-	printf("Network UPS Tools upscmd %s\n\n", UPS_VERSION);
-	printf("usage: %s [-h]\n", prog);
+	print_banner_once(prog, 2);
+	printf("NUT administration client program to initiate instant commands on UPS hardware.\n");
+
+	printf("\nusage: %s [-h]\n", prog);
 	printf("       %s [-l <ups>]\n", prog);
 	printf("       %s [-u <username>] [-p <password>] [-w] [-t <timeout>] <ups> <command> [<value>]\n\n", prog);
-	printf("Administration program to initiate instant commands on UPS hardware.\n");
 	printf("\n");
 	printf("  -h		display this help text\n");
 	printf("  -V		display the version of this software\n");
@@ -312,12 +313,11 @@ int main(int argc, char **argv)
 			break;
 
 		case 'V':
+			/* just show the version and optional
+			 * CONFIG_FLAGS banner if available */
+			print_banner_once(prog, 1);
 			nut_report_config_flags();
-
-			fatalx(EXIT_SUCCESS, "Network UPS Tools upscmd %s", UPS_VERSION);
-#ifndef HAVE___ATTRIBUTE__NORETURN
-			exit(EXIT_SUCCESS);	/* Should not get here in practice, but compiler is afraid we can fall through */
-#endif
+			exit(EXIT_SUCCESS);
 
 		case 'h':
 		default:

clients/upslog.c

@@ -144,7 +144,8 @@ static void help(const char *prog)
 
 static void help(const char *prog)
 {
-	printf("UPS status logger.\n");
+	print_banner_once(prog, 2);
+	printf("NUT read-only client program - UPS status logger.\n");
 
 	printf("\nusage: %s [OPTIONS]\n", prog);
 	printf("\n");
@@ -439,10 +440,7 @@ int main(int argc, char **argv)
 	logformat = DEFAULT_LOGFORMAT;
 	user = RUN_AS_USER;
 
-	if (!banner_is_disabled()) {
-		printf("Network UPS Tools %s %s\n", prog, UPS_VERSION);
-		fflush(stdout);
-	}
+	print_banner_once(prog, 0);
 
 	while ((i = getopt(argc, argv, "+hs:l:i:f:u:Vp:FBm:")) != -1) {
 		switch(i) {
@@ -508,6 +506,9 @@ int main(int argc, char **argv)
 				break;
 
 			case 'V':
+				/* just show the version and optional
+				 * CONFIG_FLAGS banner if available */
+				print_banner_once(prog, 1);
 				nut_report_config_flags();
 				exit(EXIT_SUCCESS);
 

clients/upsmon.c

@@ -2627,9 +2627,10 @@ static void help(const char *arg_progname)
 		/* printf("\n"); */
 	}
 
-	printf("Monitors UPS servers and may initiate shutdown if necessary.\n\n");
+	print_banner_once(arg_progname, 2);
+	printf("NUT client which monitors UPS servers and may initiate shutdown if necessary.\n");
 
-	printf("usage: %s [OPTIONS]\n\n", arg_progname);
+	printf("\nusage: %s [OPTIONS]\n\n", arg_progname);
 	printf("  -c <cmd>	send command to running process\n");
 	printf("		commands:\n");
 	printf("		 - fsd: shutdown all primary-mode UPSes (use with caution)\n");
@@ -2908,10 +2909,7 @@ int main(int argc, char *argv[])
 	}
 #endif
 
-	if (!banner_is_disabled()) {
-		printf("Network UPS Tools %s %s\n", prog, UPS_VERSION);
-		fflush(stdout);
-	}
+	print_banner_once(prog, 0);
 
 	/* if no configuration file is specified on the command line, use default */
 	configfile = xmalloc(SMALLBUF);
@@ -2973,7 +2971,9 @@ int main(int argc, char *argv[])
 				run_as_user = xstrdup(optarg);
 				break;
 			case 'V':
-				/* just show the optional CONFIG_FLAGS banner */
+				/* just show the version and optional
+				 * CONFIG_FLAGS banner if available */
+				print_banner_once(prog, 1);
 				nut_report_config_flags();
 				exit(EXIT_SUCCESS);
 			case '4':

clients/upsrw.c

@@ -47,10 +47,11 @@ struct list_t {
 
 static void usage(const char *prog)
 {
-	printf("Network UPS Tools %s %s\n\n", prog, UPS_VERSION);
-	printf("usage: %s [-h]\n", prog);
+	print_banner_once(prog, 2);
+	printf("NUT administration client program to set variables within UPS hardware.\n");
+
+	printf("\nusage: %s [-h]\n", prog);
 	printf("       %s [-s <variable>] [-u <username>] [-p <password>] [-w] [-t <timeout>] <ups>\n\n", prog);
-	printf("Demo program to set variables within UPS hardware.\n");
 	printf("\n");
 	printf("  -h            display this help text\n");
 	printf("  -V            display the version of this software\n");
@@ -668,7 +669,9 @@ int main(int argc, char **argv)
 			tracking_enabled = 1;
 			break;
 		case 'V':
-			printf("Network UPS Tools %s %s\n", prog, UPS_VERSION);
+			/* just show the version and optional
+			 * CONFIG_FLAGS banner if available */
+			print_banner_once(prog, 1);
 			nut_report_config_flags();
 			exit(EXIT_SUCCESS);
 		case 'h':

common/common.c

@@ -209,6 +209,72 @@ int banner_is_disabled(void)
 	return value;
 }
 
+const char *describe_NUT_VERSION_once(void)
+{
+	static char	buf[LARGEBUF];
+	static const char	*printed = NULL;
+
+	if (printed)
+		return printed;
+
+	memset(buf, 0, sizeof(buf));
+
+#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE
+#pragma GCC diagnostic push
+#endif
+#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE
+#pragma GCC diagnostic ignored "-Wunreachable-code"
+#endif
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunreachable-code"
+#endif
+	/* NOTE: Some compilers deduce that macro-based decisions about
+	 * NUT_VERSION_IS_RELEASE make one of codepaths unreachable in
+	 * a particular build. So we pragmatically handwave this away.
+	 */
+	if (1 < snprintf(buf, sizeof(buf),
+		"%s %s%s%s",
+		NUT_VERSION_MACRO,
+		NUT_VERSION_IS_RELEASE ? "release" : "(development iteration after ",
+		NUT_VERSION_IS_RELEASE ? "" : NUT_VERSION_SEMVER_MACRO,
+		NUT_VERSION_IS_RELEASE ? "" : ")"
+	)) {
+		printed = buf;
+	} else {
+		upslogx(LOG_WARNING, "%s: failed to report detailed NUT version", __func__);
+		printed = UPS_VERSION;
+	}
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE
+#pragma GCC diagnostic pop
+#endif
+
+	return printed;
+}
+
+int print_banner_once(const char *prog, int even_if_disabled)
+{
+	static int	printed = 0;
+	static int	ret = -1;
+
+	if (printed)
+		return ret;
+
+	if (!banner_is_disabled() || even_if_disabled) {
+		ret = printf("Network UPS Tools %s %s%s\n",
+			prog, describe_NUT_VERSION_once(),
+			even_if_disabled == 2 ? "\n" : "");
+		fflush(stdout);
+		if (ret > 0)
+			printed = 1;
+	}
+
+	return ret;
+}
+
 /* enable writing upslog_with_errno() and upslogx() type messages to
    the syslog */
 void syslogbit_set(void)
@@ -2195,28 +2261,11 @@ void nut_report_config_flags(void)
 		now.tv_sec -= 1;
 	}
 
-#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE
-#pragma GCC diagnostic push
-#endif
-#ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE
-#pragma GCC diagnostic ignored "-Wunreachable-code"
-#endif
-#ifdef __clang__
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wunreachable-code"
-#endif
-	/* NOTE: Some compilers deduce that macro-based decisions about
-	 * NUT_VERSION_IS_RELEASE make one of codepaths unreachable in
-	 * a particular build. So we pragmatically handwave this away.
-	 */
 	if (xbit_test(upslog_flags, UPSLOG_STDERR)) {
-		fprintf(stderr, "%4.0f.%06ld\t[D1] Network UPS Tools version %s%s%s%s%s%s%s %s%s\n",
+		fprintf(stderr, "%4.0f.%06ld\t[D1] Network UPS Tools version %s%s%s%s %s%s\n",
 			difftime(now.tv_sec, upslog_start.tv_sec),
 			(long)(now.tv_usec - upslog_start.tv_usec),
-			UPS_VERSION,
-			NUT_VERSION_IS_RELEASE ? " release" : " (development iteration after ",
-			NUT_VERSION_IS_RELEASE ? "" : NUT_VERSION_SEMVER_MACRO,
-			NUT_VERSION_IS_RELEASE ? "" : ")",
+			describe_NUT_VERSION_once(),
 			(compiler_ver && *compiler_ver != '\0' ? " built with " : ""),
 			(compiler_ver && *compiler_ver != '\0' ? compiler_ver : ""),
 			(compiler_ver && *compiler_ver != '\0' ? " and" : ""),
@@ -2231,24 +2280,15 @@ void nut_report_config_flags(void)
 	/* NOTE: May be ignored or truncated by receiver if that syslog server
 	 * (and/or OS sender) does not accept messages of such length */
 	if (xbit_test(upslog_flags, UPSLOG_SYSLOG)) {
-		syslog(LOG_DEBUG, "Network UPS Tools version %s%s%s%s%s%s%s %s%s",
-			UPS_VERSION,
-			NUT_VERSION_IS_RELEASE ? " release" : " (development iteration after ",
-			NUT_VERSION_IS_RELEASE ? "" : NUT_VERSION_SEMVER_MACRO,
-			NUT_VERSION_IS_RELEASE ? "" : ")",
+		syslog(LOG_DEBUG, "Network UPS Tools version %s%s%s%s %s%s",
+			describe_NUT_VERSION_once(),
 			(compiler_ver && *compiler_ver != '\0' ? " built with " : ""),
 			(compiler_ver && *compiler_ver != '\0' ? compiler_ver : ""),
 			(compiler_ver && *compiler_ver != '\0' ? " and" : ""),
 			(config_flags && *config_flags != '\0' ? "configured with flags: " : "configured all by default guesswork"),
 			(config_flags && *config_flags != '\0' ? config_flags : "")
 		);
 	}
-#ifdef __clang__
-#pragma clang diagnostic pop
-#endif
-#ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_UNREACHABLE_CODE
-#pragma GCC diagnostic pop
-#endif
 }
 
 static void vupslog(int priority, const char *fmt, va_list va, int use_strerror)