From 6f1e257978f860d00c4c1a81c6f75a489a532276 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 31 May 2024 15:49:21 +0200 Subject: [PATCH] common/common.c, tests/nutlogtest.c: minimize_formatting_string(): collapse known different formats for same data type into same char to ease sanity-check comparisons [#2450] Signed-off-by: Jim Klimov --- common/common.c | 97 ++++++++++++++++++++++++++++++++-------------- tests/nutlogtest.c | 26 +++++++++++++ 2 files changed, 94 insertions(+), 29 deletions(-) diff --git a/common/common.c b/common/common.c index c35d40ed5c..19b9c435d1 100644 --- a/common/common.c +++ b/common/common.c @@ -1346,31 +1346,77 @@ char * minimize_formatting_string(char *buf, size_t buflen, const char *fmt) * Or another character that is critical for stack * intepretation as a variable argument list? * https://cplusplus.com/reference/cstdio/printf/ + * Note that some widths are only required with + * C99 and C++11 standards, and may be not available + * before. Still, since we rely on macro names from + * those standards (or inspired by them) like PRIiMAX + * or PRIuSIZE, defined (if missing in OS headers) + * by our "nut_stdint.h". */ - - 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; + switch (*p) { + /* We care about integer/pointer type size "width" modifiers, e.g.: */ + case 'l': /* long (long) int */ + case 'L': /* long double */ + case 'h': /* short int/char */ +#if defined(__cplusplus) || (defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)) || (defined _STDC_C99) || (defined __C99FEATURES__) /* C99+ build mode */ || (defined PRIiMAX && defined PRIiSIZE) + /* Technically, double "ll" and "hh" are also new */ + case 'z': /* size_t */ + case 'j': /* intmax_t */ + case 't': /* ptrdiff_t */ +#endif + /* and here field length will be in another vararg on the stack: */ + case '*': + *b++ = *p; + i++; + continue; + + /* Known conversion characters, collapse some numeric format + * specifiers to unambiguous basic type for later comparisons */ + case 'd': + case 'i': + inEscape = 0; + *b++ = 'i'; + i++; + continue; + + case 'u': + case 'o': + case 'x': + case 'X': + inEscape = 0; + *b++ = 'u'; + i++; + continue; + + case 'f': + case 'e': + case 'E': + case 'g': + case 'G': +#if defined(__cplusplus) || (defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)) || (defined _STDC_C99) || (defined __C99FEATURES__) /* C99+ build mode */ + case 'F': + case 'a': + case 'A': +#endif + inEscape = 0; + *b++ = 'f'; + i++; + continue; + + case 'c': + case 's': + case 'p': + case 'n': + inEscape = 0; + *b++ = *p; + i++; + continue; } - if ((*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z') || *p == '*') { - /* Assume a conversion character (standards-dependent) */ - inEscape = 0; - *b++ = *p; - i++; - } + /* Assume and skip a flags/width/precision character + * (non-POSIX standards-dependent), inconsequential + * for printf style stack parsing and memory-safety. + */ } } @@ -1455,13 +1501,6 @@ int validate_formatting_string(const char *fmt_dynamic, const char *fmt_referenc return 0; } - /* 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); diff --git a/tests/nutlogtest.c b/tests/nutlogtest.c index 1e5139f461..d46d9b5dd5 100644 --- a/tests/nutlogtest.c +++ b/tests/nutlogtest.c @@ -70,6 +70,7 @@ int main(void) { char *dynfmt = "Test '%s'", *p; char buf[LARGEBUF]; + nut_debug_level = 1; if (snprintf_dynamic(buf, sizeof(buf), dynfmt, "%s", "Single string via dynamic format") > 0) { upsdebugx(0, "D: >>> %s", buf); if (!strcmp(buf, "Test 'Single string via dynamic format'")) { @@ -95,10 +96,35 @@ int main(void) { upsdebugx(0, ">>> %s", NUT_STRARG(p)); if (!p || *p == '\0' || strcmp(p, "Test 'Single string inlined by mkstr_dynamic()'")) { upsdebugx(0, "E: mkstr_dynamic() failed to prepare a dynamically formatted string: got unexpected content"); + ret++; } else { upsdebugx(0, "D: mkstr_dynamic() prepared a dynamically formatted string with expected content"); } } + if (1) { /* scoping */ + char **p, + *fmtFloat[] = { "%f", " %A", " %0.1E%% ", NULL }, + *fmtNotFloat[] = { "%f%", "%m", "$f", NULL }; + + for (p = &(fmtFloat[0]); *p; p++) { + if (validate_formatting_string(*p, "Voltage: %G is not %%d") < 0) { + upsdebugx(0, "E: validate_formatting_string() expecting %%f equivalent failed for: '%s'", *p); + ret++; + } else { + upsdebugx(0, "D: validate_formatting_string() expecting %%f equivalent passed for: '%s'", *p); + } + } + + for (p = &(fmtNotFloat[0]); *p; p++) { + if (validate_formatting_string("%f", *p) < 0) { + upsdebugx(0, "D: validate_formatting_string() expecting %%f failed (as it should have) for: '%s'", *p); + } else { + upsdebugx(0, "E: validate_formatting_string() expecting %%f passed (but should not have) for: '%s'", *p); + ret++; + } + } + } + return ret; }