From 98498507add9c74bc6d1e1adf3c28569ef292863 Mon Sep 17 00:00:00 2001
From: Jim Klimov <jimklimov+nut@gmail.com>
Date: Fri, 31 May 2024 15:06:59 +0200
Subject: [PATCH] fixup! common/common.c, include/common.h, tests/nutlogtest.c,
 NEWS.adoc: introduce snprintf_dynamic() and related methods [#2450]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
---
 common/common.c | 52 +++++++++++++++++++++++++++++++++++++------------
 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/common/common.c b/common/common.c
index 682da2f418..c35d40ed5c 100644
--- a/common/common.c
+++ b/common/common.c
@@ -1331,7 +1331,7 @@ char * minimize_formatting_string(char *buf, size_t buflen, const char *fmt)
 	) {
 		if (*p == '%') {
 			if (*(p+1) == '%') {
-				/* Escaped percent character, not a variable indicator */
+				/* Escaped percent character, not a variable indicator; skip it right away */
 				p++;
 			} else {
 				inEscape = 1;
@@ -1342,10 +1342,31 @@ char * minimize_formatting_string(char *buf, size_t buflen, const char *fmt)
 		}
 
 		if (inEscape) {
-			/* Did we hit a printf format character? */
-			if (*p == '%')
+			/* Did we hit a printf format conversion character?
+			 * Or another character that is critical for stack
+			 * intepretation as a variable argument list?
+			 * https://cplusplus.com/reference/cstdio/printf/
+			 */
+
+			if (*p == 'l' || *p == 'L' || *p == 'h' || *p == 'z' || *p == 'j' || *p == 't') {
+				/* Integer/pointer type size modifiers, e.g.
+				 * long (long...) something, or a short int (h)
+				 * size_t (z), intmax_t (j), ptrdiff_t (t)
+				 */
+				*b++ = *p;
+				i++;
+				continue;
+			}
+
+			if (*p == '*') {
+				/* Field length will be in another vararg on the stack */
+				*b++ = *p;
+				i++;
 				continue;
-			if ((*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z')) {
+			}
+
+			if ((*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z') || *p == '*') {
+				/* Assume a conversion character (standards-dependent) */
 				inEscape = 0;
 				*b++ = *p;
 				i++;
@@ -1427,20 +1448,27 @@ int validate_formatting_string(const char *fmt_dynamic, const char *fmt_referenc
 			return -2;
 		}
 
-		if (strcmp(bufD, bufR)) {
-			/* This be should not be fatal right here, but may be in the caller logic */
-			upsdebugx(1, "%s: dynamic formatting string '%s' is not equivalent to expected '%s'",
-				__func__, fmt_dynamic, fmt_reference);
+		if (!strcmp(bufD, bufR)) {
+			/* Two strings compared as equals, good to go */
 			free(bufD);
 			free(bufR);
-			errno = EINVAL;
-			return -3;
+			return 0;
 		}
 
-		/* Two strings compared as equals, good to go */
+		/* FIXME: Check for functional equivalence, e.g. format chars
+		 * "x", "X", "i", "d", "u" ("o", "c"?) all describe an int,
+		 * and "g", "G", "f", "F", "E" are doubles.
+		 * Cross-check with standards about automatic expansion of
+		 * byte-size for many of these types when passed in varargs.
+		 */
+
+		/* This be should not be fatal right here, but may be in the caller logic */
+		upsdebugx(1, "%s: dynamic formatting string '%s' is not equivalent to expected '%s'",
+			__func__, fmt_dynamic, fmt_reference);
 		free(bufD);
 		free(bufR);
-		return 0;
+		errno = EINVAL;
+		return -3;
 	}
 }