QtTest: port from qsnprintf to std::snprintf and mark the module as qsnprintf-free

Drive-by remove an explicit NUL-termination (std::snprintf() does
that) and port a repeated use of printf argument checking to the
protect() idiom.

Change-Id: Ida15940fe9aef0622e9836a229a398c909503a9a
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit b5115d1c2fc73feb149a8ee97de011b3c75694fb)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/testlib/CMakeLists.txt

@@ -68,6 +68,7 @@ qt_internal_add_module(Test
         QT_NO_CONTEXTLESS_CONNECT
         QT_NO_DATASTREAM
         QT_NO_FOREACH
+        QT_NO_QSNPRINTF
         QT_USE_NODISCARD_FILE_OPEN
         # Ensure uniform location info between release and debug builds
         QT_NO_MESSAGELOGCONTEXT

src/testlib/qabstracttestlogger.cpp

@@ -9,6 +9,8 @@
 #include <QtCore/qbytearray.h>
 #include <QtCore/qstring.h>
 
+#include <cstdio>
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
@@ -405,7 +407,7 @@ int qt_asprintf(QTestCharBuffer *str, const char *format, ...)
 
     do {
         va_start(ap, format);
-        res = qvsnprintf(str->data(), size, format, ap);
+        res = std::vsnprintf(str->data(), size, format, ap);
         va_end(ap);
         // vsnprintf() reliably '\0'-terminates
         Q_ASSERT(res < 0 || str->data()[res < size ? res : size - 1] == '\0');

src/testlib/qcsvbenchmarklogger.cpp

@@ -5,6 +5,8 @@
 #include "qtestresult_p.h"
 #include "qbenchmark_p.h"
 
+#include <cstdio>
+
 /*! \internal
     \class QCsvBenchmarkLogger
     \inmodule QtTest
@@ -61,7 +63,7 @@ void QCsvBenchmarkLogger::addBenchmarkResult(const QBenchmarkResult &result)
 
     char buf[1024];
     // "function","[globaltag:]tag","metric",value_per_iteration,total,iterations
-    qsnprintf(buf, sizeof(buf), "\"%s\",\"%s%s%s\",\"%s\",%.13g,%.13g,%u\n",
+    std::snprintf(buf, sizeof(buf), "\"%s\",\"%s%s%s\",\"%s\",%.13g,%.13g,%u\n",
                   fn, gtag, filler, tag, metric,
                   result.measurement.value / result.iterations,
                   result.measurement.value, result.iterations);

src/testlib/qjunittestlogger.cpp

@@ -11,6 +11,8 @@
 
 #include <QtCore/qlibraryinfo.h>
 
+#include <cstdio>
+
 #include <string.h>
 
 QT_BEGIN_NAMESPACE
@@ -93,16 +95,16 @@ void QJUnitTestLogger::stopLogging()
 {
     char buf[10];
 
-    qsnprintf(buf, sizeof(buf), "%i", testCounter);
+    std::snprintf(buf, sizeof(buf), "%i", testCounter);
     currentTestSuite->addAttribute(QTest::AI_Tests, buf);
 
-    qsnprintf(buf, sizeof(buf), "%i", failureCounter);
+    std::snprintf(buf, sizeof(buf), "%i", failureCounter);
     currentTestSuite->addAttribute(QTest::AI_Failures, buf);
 
-    qsnprintf(buf, sizeof(buf), "%i", errorCounter);
+    std::snprintf(buf, sizeof(buf), "%i", errorCounter);
     currentTestSuite->addAttribute(QTest::AI_Errors, buf);
 
-    qsnprintf(buf, sizeof(buf), "%i", QTestLog::skipCount());
+    std::snprintf(buf, sizeof(buf), "%i", QTestLog::skipCount());
     currentTestSuite->addAttribute(QTest::AI_Skipped, buf);
 
     currentTestSuite->addAttribute(QTest::AI_Time,

src/testlib/qplaintestlogger.cpp

@@ -11,6 +11,7 @@
 #include <QtCore/private/qlogging_p.h>
 
 #include <array>
+#include <cstdio>
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -67,7 +68,7 @@ template <int N> struct FixedBufString
     template <typename... Args> void appendf(const char *format, Args... args)
     {
         // vsnprintf includes the terminating null
-        used += qsnprintf(buf.data() + used, MaxSize - used + 1, format,
+        used += std::snprintf(buf.data() + used, MaxSize - used + 1, format,
                               args...);
     }
 
@@ -413,9 +414,9 @@ void QPlainTestLogger::startLogging()
 
     char buf[1024];
     if (QTestLog::verboseLevel() < 0) {
-        qsnprintf(buf, sizeof(buf), "Testing %s\n", QTestResult::currentTestObjectName());
+        std::snprintf(buf, sizeof(buf), "Testing %s\n", QTestResult::currentTestObjectName());
     } else {
-        qsnprintf(buf, sizeof(buf),
+        std::snprintf(buf, sizeof(buf),
                       "********* Start testing of %s *********\n"
                       "Config: Using QtTest library " QTEST_VERSION_STR
                       ", %s, %s %s\n", QTestResult::currentTestObjectName(), QLibraryInfo::build(),
@@ -429,11 +430,12 @@ void QPlainTestLogger::stopLogging()
     char buf[1024];
     const int timeMs = qRound(QTestLog::msecsTotalTime());
     if (QTestLog::verboseLevel() < 0) {
-        qsnprintf(buf, sizeof(buf), "Totals: %d passed, %d failed, %d skipped, %d blacklisted, %dms\n",
+        std::snprintf(buf, sizeof(buf),
+                      "Totals: %d passed, %d failed, %d skipped, %d blacklisted, %dms\n",
                       QTestLog::passCount(), QTestLog::failCount(),
                       QTestLog::skipCount(), QTestLog::blacklistCount(), timeMs);
     } else {
-        qsnprintf(buf, sizeof(buf),
+        std::snprintf(buf, sizeof(buf),
                       "Totals: %d passed, %d failed, %d skipped, %d blacklisted, %dms\n"
                       "********* Finished testing of %s *********\n",
                       QTestLog::passCount(), QTestLog::failCount(),

src/testlib/qtestcase.cpp

@@ -61,6 +61,7 @@
 #endif
 #include <chrono>
 #include <cmath>
+#include <cstdio>
 #include <limits>
 #include <memory>
 #include <mutex>
@@ -1349,7 +1350,7 @@ bool TestMethods::invokeTest(int index, QLatin1StringView tag, std::optional<Wat
             QTestResult::setCurrentGlobalTestData(gTable->testData(curGlobalDataIndex));
 
         if (curGlobalDataIndex == 0) {
-            qsnprintf(member, 512, "%s_data()", name.constData());
+            std::snprintf(member, 512, "%s_data()", name.constData());
             invokeTestMethodIfExists(member);
             if (QTestResult::skipCurrentTest())
                 break;
@@ -2587,9 +2588,10 @@ QTestData &QTest::newRow(const char *dataTag)
 
     Appends a new row to the current test data.
 
-    The function's arguments are passed to qsnprintf() for formatting according
+    The function's arguments are passed to std::snprintf() for
-    to \a format. See the qvsnprintf() documentation for caveats and
+    formatting according to \a format. See the
-    limitations.
+    \l{https://en.cppreference.com/w/cpp/io/c/fprintf}{std::snprintf()
+    documentation} for caveats and limitations.
 
     The test output will identify the test run with this test data using the
     name that results from this formatting.
@@ -2622,8 +2624,7 @@ QTestData &QTest::addRow(const char *format, ...)
     va_start(va, format);
     // we don't care about failures, we accept truncation, as well as trailing garbage.
     // Names with more than 1K characters are nonsense, anyway.
-    (void)qvsnprintf(buf, sizeof buf, format, va);
+    std::vsnprintf(buf, sizeof buf, format, va);
-    buf[sizeof buf - 1] = '\0';
     va_end(va);
 
     return *tbl->newData(buf);
@@ -2996,7 +2997,7 @@ bool QTest::qCompare(const QLatin1StringView &t1, QStringView t2, const char *ac
 template <> Q_TESTLIB_EXPORT char *QTest::toString<TYPE>(const TYPE &t) \
 { \
     char *msg = new char[128]; \
-    qsnprintf(msg, 128, #FORMAT, t); \
+    std::snprintf(msg, 128, #FORMAT, t); \
     return msg; \
 }
 
@@ -3053,7 +3054,7 @@ template <> Q_TESTLIB_EXPORT char *QTest::toString<TYPE>(const TYPE &t) \
         qstrncpy(msg, "nan", 128); \
         break; \
     default: \
-        qsnprintf(msg, 128, #FORMAT, double(t));    \
+        std::snprintf(msg, 128, #FORMAT, double(t));    \
         massageExponent(msg); \
         break; \
     } \
@@ -3104,9 +3105,9 @@ template <> Q_TESTLIB_EXPORT char *QTest::toString<char>(const char &t)
         break;
     default:
         if (c < 0x20 || c >= 0x7F)
-            qsnprintf(msg, 16, "'\\x%02x'", c);
+            std::snprintf(msg, 16, "'\\x%02x'", c);
         else
-            qsnprintf(msg, 16, "'%c'" , c);
+            std::snprintf(msg, 16, "'%c'" , c);
     }
     return msg;
 }
@@ -3129,7 +3130,7 @@ char *QTest::toString(const char *str)
 char *QTest::toString(const volatile void *p) // Use volatile to match compare_ptr_helper()
 {
     char *msg = new char[128];
-    qsnprintf(msg, 128, "%p", p);
+    std::snprintf(msg, 128, "%p", p);
     return msg;
 }
 
@@ -3154,9 +3155,9 @@ char *QTest::toString(const QObject *o)
     const char *className = o->metaObject()->className();
     char *msg = new char[256];
     if (name.isEmpty())
-        qsnprintf(msg, 256, "%s/%p", className, o);
+        std::snprintf(msg, 256, "%s/%p", className, o);
     else
-        qsnprintf(msg, 256, "%s/\"%s\"", className, qPrintable(name));
+        std::snprintf(msg, 256, "%s/\"%s\"", className, qPrintable(name));
     return msg;
 }
 

src/testlib/qtestlog.cpp

@@ -30,6 +30,8 @@
 #include <QtCore/QRegularExpression>
 #endif
 
+#include <cstdio>
+
 #include <stdlib.h>
 #include <string.h>
 #include <limits.h>
@@ -194,7 +196,7 @@ namespace QTest {
 
             const size_t maxMsgLen = 1024;
             char msg[maxMsgLen] = {'\0'};
-            qsnprintf(msg, maxMsgLen, "Received a warning that resulted in a failure:\n%s",
+            std::snprintf(msg, maxMsgLen, "Received a warning that resulted in a failure:\n%s",
                           qPrintable(message));
             QTestResult::addFailure(msg, context.file, context.line);
             return true;

src/testlib/qtestresult.cpp

@@ -307,12 +307,12 @@ bool QTestResult::verify(bool statement, const char *statementStr,
     msg[0] = '\0';
 
     if (QTestLog::verboseLevel() >= 2) {
-        qsnprintf(msg, maxMsgLen, "QVERIFY(%s)", statementStr);
+        std::snprintf(msg, maxMsgLen, "QVERIFY(%s)", statementStr);
         QTestLog::info(msg, file, line);
     }
 
     if (statement == !!QTest::expectFailMode) {
-        qsnprintf(msg, maxMsgLen,
+        std::snprintf(msg, maxMsgLen,
                       statement ? "'%s' returned TRUE unexpectedly. (%s)" : "'%s' returned FALSE. (%s)",
                       statementStr, description ? description : "");
     }
@@ -351,19 +351,21 @@ void formatFailMessage(char *msg, size_t maxMsgLen,
 {
     const auto len1 = approx_wide_len(actual);
     const auto len2 = approx_wide_len(expected);
-    const int written = qsnprintf(msg, maxMsgLen, "%s\n", failureMsg);
+    const int written = std::snprintf(msg, maxMsgLen, "%s\n", failureMsg);
     msg += written;
     maxMsgLen -= written;
 
+    const auto protect = [](const char *s) { return s ? s : "<null>"; };
+
     if (val1 || val2) {
-        qsnprintf(msg, maxMsgLen, "   %s(%s)%*s %s\n   %s(%s)%*s %s",
+        std::snprintf(msg, maxMsgLen, "   %s(%s)%*s %s\n   %s(%s)%*s %s",
                       leftArgNameForOp(op), actual, qMax(len1, len2) - len1 + 1, ":",
-                    val1 ? val1 : "<null>",
+                      protect(val1),
                       rightArgNameForOp(op), expected, qMax(len1, len2) - len2 + 1, ":",
-                    val2 ? val2 : "<null>");
+                      protect(val2));
     } else {
         // only print variable names if neither value can be represented as a string
-        qsnprintf(msg, maxMsgLen, "   %s: %s\n   %s: %s",
+        std::snprintf(msg, maxMsgLen, "   %s: %s\n   %s: %s",
                       leftArgNameForOp(op), actual, rightArgNameForOp(op), expected);
     }
 }
@@ -411,7 +413,7 @@ static bool compareHelper(bool success, const char *failureMsg,
     QTEST_ASSERT(actual);
 
     if (QTestLog::verboseLevel() >= 2) {
-        qsnprintf(msg, maxMsgLen, "QCOMPARE(%s, %s)", actual, expected);
+        std::snprintf(msg, maxMsgLen, "QCOMPARE(%s, %s)", actual, expected);
         QTestLog::info(msg, file, line);
     }
 
@@ -420,7 +422,7 @@ static bool compareHelper(bool success, const char *failureMsg,
 
     if (success) {
         if (QTest::expectFailMode) {
-            qsnprintf(msg, maxMsgLen,
+            std::snprintf(msg, maxMsgLen,
                           "QCOMPARE(%s, %s) returned TRUE unexpectedly.", actual, expected);
         }
         return checkStatement(success, msg, file, line);
@@ -428,7 +430,7 @@ static bool compareHelper(bool success, const char *failureMsg,
 
 
     if (!hasValues) {
-        qsnprintf(msg, maxMsgLen, "%s", failureMsg);
+        std::snprintf(msg, maxMsgLen, "%s", failureMsg);
         return checkStatement(success, msg, file, line);
     }
 
@@ -455,13 +457,13 @@ static bool compareHelper(bool success, const char *failureMsg,
     QTEST_ASSERT(success || failureMsg);
 
     if (QTestLog::verboseLevel() >= 2) {
-        qsnprintf(msg, maxMsgLen, "QCOMPARE(%s, %s)", actual, expected);
+        std::snprintf(msg, maxMsgLen, "QCOMPARE(%s, %s)", actual, expected);
         QTestLog::info(msg, file, line);
     }
 
     if (success) {
         if (QTest::expectFailMode) {
-            qsnprintf(msg, maxMsgLen, "QCOMPARE(%s, %s) returned TRUE unexpectedly.",
+            std::snprintf(msg, maxMsgLen, "QCOMPARE(%s, %s) returned TRUE unexpectedly.",
                           actual, expected);
         }
         return checkStatement(success, msg, file, line);
@@ -671,13 +673,13 @@ bool QTestResult::reportResult(bool success, const void *lhs, const void *rhs,
     QTEST_ASSERT(rhsExpr);
 
     if (QTestLog::verboseLevel() >= 2) {
-        qsnprintf(msg, maxMsgLen, "%s(%s, %s)", macroNameForOp(op), lhsExpr, rhsExpr);
+        std::snprintf(msg, maxMsgLen, "%s(%s, %s)", macroNameForOp(op), lhsExpr, rhsExpr);
         QTestLog::info(msg, file, line);
     }
 
     if (success) {
         if (QTest::expectFailMode) {
-            qsnprintf(msg, maxMsgLen, "%s(%s, %s) returned TRUE unexpectedly.",
+            std::snprintf(msg, maxMsgLen, "%s(%s, %s) returned TRUE unexpectedly.",
                           macroNameForOp(op), lhsExpr, rhsExpr);
         }
         return checkStatement(success, msg, file, line);