From 0eab6aac051671c0aaf4a87ba6ec8926a5ec2377 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 18 Jul 2024 10:53:42 +0200 Subject: [PATCH] Introduce QT_NO_QSNPRINTF and mark QtCore as qsnprintf-free ... except, of course, the implementation file, which therefore has to be excluded from PCH'ed builds. Remove qvsnprintf.cpp from libbootstrap, as it's no longer needed after porting all five uses of qsnprintf() in QtCore to C++11's std::snprintf(), which even Windows implements with std behavior. The reason we're removing the function is that std::snprintf() is the better alternative: qsnprintf() just introduces even more platform variance than any implementation-defined behavior of C or C++ snprintf(). To wit: - On Windows, the return value is sometimes "wrong" (cf. Windows-specific qsnprintf() tests in tst_qbytearray.cpp) - On WASM and at least some Android configurations, it is incorrectly implmented using the QString::asprintf().toLocal8Bit() work-around, even though both platforms have a working snprintf(). QString::asprintf() is implemented in Qt itself, so has some differences: - the `a` format for hex floats is not supported - %ls expects char16_t*, not wchar_t* (these are, in general, not the same width) We will deprecate these functions in 6.9, but before we do, we need to get the Qt code in order, and that's where this macro comes in. [ChangeLog][QtCore] Added the QT_NO_QSNPRINTF macro to disable qsnprintf() and qvsnprintf(), which will also be deprecated in 6.9. See the documentation for details why we take this step. Task-number: QTBUG-127110 Change-Id: I4e1c1f213bcfd615f83387f5f51e77fa1ff2062e Reviewed-by: Volker Hilsheimer (cherry picked from commit bd7d54249e3f2b6a9dd6b759c892d7c97d26c0aa) Reviewed-by: Ahmad Samir Reviewed-by: Marc Mutz --- src/corelib/CMakeLists.txt | 1 + src/corelib/global/qglobal.cpp | 1 + src/corelib/global/qtconfigmacros.h | 4 ++++ src/corelib/io/qdebug.cpp | 5 +++-- src/corelib/io/qfilesystemengine_win.cpp | 10 ++++++---- src/corelib/kernel/qeventdispatcher_unix.cpp | 4 +++- src/corelib/text/qbytearrayalgorithms.h | 2 ++ src/corelib/text/qlocale_tools.cpp | 5 ++++- src/corelib/text/qvsnprintf.cpp | 17 +++++++++++++++++ src/tools/bootstrap/CMakeLists.txt | 1 - 10 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index de28a6183e9..320f9e20a5d 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -332,6 +332,7 @@ qt_internal_add_module(Core QT_NO_CONTEXTLESS_CONNECT QT_NO_FOREACH QT_NO_QPAIR + QT_NO_QSNPRINTF QT_NO_USING_NAMESPACE QT_TYPESAFE_FLAGS QT_USE_NODISCARD_FILE_OPEN diff --git a/src/corelib/global/qglobal.cpp b/src/corelib/global/qglobal.cpp index 6ce7f97580b..537bcc16c59 100644 --- a/src/corelib/global/qglobal.cpp +++ b/src/corelib/global/qglobal.cpp @@ -361,6 +361,7 @@ bool QInternal::activateCallbacks(Callback cb, void **parameters) \row \li 6.7.0 \li Overloads of QObject::connect that do not take a context object (see \l{QT_NO_CONTEXTLESS_CONNECT}) \row \li 6.8.0 \li The qAsConst() function (see \l{QT_NO_QASCONST}) \row \li 6.8.0 \li File-related I/O classes have their \c{open()} functions marked \c{[[nodiscard]]} (see \l{QT_USE_NODISCARD_FILE_OPEN}) + \row\li 6.9.0 \li The qsnprintf() and qvnprintf() functions (see \l{QT_NO_QSNPRINTF}). \endtable Moreover, individual APIs may also get disabled as part of the diff --git a/src/corelib/global/qtconfigmacros.h b/src/corelib/global/qtconfigmacros.h index b643122d5c1..c2001efac3e 100644 --- a/src/corelib/global/qtconfigmacros.h +++ b/src/corelib/global/qtconfigmacros.h @@ -205,6 +205,10 @@ namespace QT_NAMESPACE {} # define QT_USE_NODISCARD_FILE_OPEN # endif #endif // 6.8.0 + +#if QT_ENABLE_STRICT_MODE_UP_TO >= QT_VERSION_CHECK(6, 9, 0) +# define QT_NO_QSNPRINTF +#endif // 6.9.0 #endif // QT_ENABLE_STRICT_MODE_UP_TO #endif /* QTCONFIGMACROS_H */ diff --git a/src/corelib/io/qdebug.cpp b/src/corelib/io/qdebug.cpp index 600b8b13b80..e9b58855f5d 100644 --- a/src/corelib/io/qdebug.cpp +++ b/src/corelib/io/qdebug.cpp @@ -11,6 +11,7 @@ #include #include +#include QT_BEGIN_NAMESPACE @@ -406,9 +407,9 @@ static QByteArray timeUnit(qint64 num, qint64 den) }; auto appendNumber = [&](qint64 value) { if (value >= 10'000 && (value % 1000) == 0) - len += qsnprintf(buf + len, sizeof(buf) - len, "%.6g", double(value)); // "1e+06" + len += std::snprintf(buf + len, sizeof(buf) - len, "%.6g", double(value)); // "1e+06" else - len += qsnprintf(buf + len, sizeof(buf) - len, "%lld", value); + len += std::snprintf(buf + len, sizeof(buf) - len, "%lld", value); }; appendChar('['); appendNumber(num); diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp index 3ec32e31a12..bdcac20dd32 100644 --- a/src/corelib/io/qfilesystemengine_win.cpp +++ b/src/corelib/io/qfilesystemengine_win.cpp @@ -37,6 +37,8 @@ #define SECURITY_WIN32 #include +#include + #include #ifndef SPI_GETPLATFORMTYPE @@ -1010,10 +1012,10 @@ static inline QByteArray fileId(HANDLE handle) BY_HANDLE_FILE_INFORMATION info; if (GetFileInformationByHandle(handle, &info)) { char buffer[sizeof "01234567:0123456701234567"]; - qsnprintf(buffer, sizeof(buffer), "%lx:%08lx%08lx", - info.dwVolumeSerialNumber, - info.nFileIndexHigh, - info.nFileIndexLow); + std::snprintf(buffer, sizeof(buffer), "%lx:%08lx%08lx", + info.dwVolumeSerialNumber, + info.nFileIndexHigh, + info.nFileIndexLow); return buffer; } return QByteArray(); diff --git a/src/corelib/kernel/qeventdispatcher_unix.cpp b/src/corelib/kernel/qeventdispatcher_unix.cpp index 21bd2244157..1b7a6537b05 100644 --- a/src/corelib/kernel/qeventdispatcher_unix.cpp +++ b/src/corelib/kernel/qeventdispatcher_unix.cpp @@ -14,6 +14,8 @@ #include #include +#include + #include #include #include @@ -87,7 +89,7 @@ bool QThreadPipe::init() #if defined(Q_OS_WASM) // do nothing. #elif defined(Q_OS_VXWORKS) - qsnprintf(name, sizeof(name), "/pipe/qt_%08x", int(taskIdSelf())); + std::snprintf(name, sizeof(name), "/pipe/qt_%08x", int(taskIdSelf())); // make sure there is no pipe with this name pipeDevDelete(name, true); diff --git a/src/corelib/text/qbytearrayalgorithms.h b/src/corelib/text/qbytearrayalgorithms.h index 5b2b71838fa..7b9529eba95 100644 --- a/src/corelib/text/qbytearrayalgorithms.h +++ b/src/corelib/text/qbytearrayalgorithms.h @@ -138,11 +138,13 @@ Q_CORE_EXPORT int qstricmp(const char *, const char *); Q_CORE_EXPORT int qstrnicmp(const char *, const char *, size_t len); Q_CORE_EXPORT int qstrnicmp(const char *, qsizetype, const char *, qsizetype = -1); +#ifndef QT_NO_QSNPRINTF // use std::(v)snprintf() from instead // implemented in qvsnprintf.cpp Q_CORE_EXPORT int qvsnprintf(char *str, size_t n, const char *fmt, va_list ap) Q_ATTRIBUTE_FORMAT_PRINTF(3, 0); Q_CORE_EXPORT int qsnprintf(char *str, size_t n, const char *fmt, ...) Q_ATTRIBUTE_FORMAT_PRINTF(3, 4); +#endif // QT_NO_QSNPRINTF // qChecksum: Internet checksum Q_CORE_EXPORT quint16 qChecksum(QByteArrayView data, Qt::ChecksumType standard = Qt::ChecksumIso3309); diff --git a/src/corelib/text/qlocale_tools.cpp b/src/corelib/text/qlocale_tools.cpp index 6b04bc92cd1..00d473099d6 100644 --- a/src/corelib/text/qlocale_tools.cpp +++ b/src/corelib/text/qlocale_tools.cpp @@ -10,6 +10,8 @@ #include #include +#include + #include #include #include @@ -324,7 +326,8 @@ QSimpleParsedNumber qt_asciiToDouble(const char *num, qsizetype numLen, constexpr auto maxDigitsForULongLong = 1 + std::numeric_limits::digits10; // need to ensure that we don't read more than numLen of input: char fmt[1 + maxDigitsForULongLong + 4 + 1]; - qsnprintf(fmt, sizeof fmt, "%s%llu%s", "%", static_cast(numLen), "lf%n"); + std::snprintf(fmt, sizeof fmt, "%s%llu%s", + "%", static_cast(numLen), "lf%n"); if (qDoubleSscanf(num, QT_CLOCALE, fmt, &d, &processed) < 1) processed = 0; diff --git a/src/corelib/text/qvsnprintf.cpp b/src/corelib/text/qvsnprintf.cpp index dd988161d53..afe526516ff 100644 --- a/src/corelib/text/qvsnprintf.cpp +++ b/src/corelib/text/qvsnprintf.cpp @@ -10,9 +10,22 @@ QT_BEGIN_NAMESPACE +/*! + \macro QT_NO_QSNPRINTF + \since 6.8 + \relates QByteArray + + Defining this macro removes the availability of the qsnprintf() and + qvsnprintf() functions. See the functions' documentation for why you may + want to disable them. + + \sa qsnprintf(), qvsnprintf(). +*/ + #if !defined(QT_VSNPRINTF) || defined(Q_QDOC) /*! + \fn int qvsnprintf(char *str, size_t n, const char *fmt, va_list ap) \relates QByteArray \obsolete @@ -40,6 +53,7 @@ QT_BEGIN_NAMESPACE \sa qsnprintf(), QString::asprintf() */ +Q_CORE_EXPORT // QT_NO_QSNPRINTF is in effect int qvsnprintf(char *str, size_t n, const char *fmt, va_list ap) { if (!str || !fmt) @@ -62,6 +76,7 @@ QT_BEGIN_INCLUDE_NAMESPACE #include QT_END_INCLUDE_NAMESPACE +Q_CORE_EXPORT // QT_NO_QSNPRINTF is in effect int qvsnprintf(char *str, size_t n, const char *fmt, va_list ap) { return QT_VSNPRINTF(str, n, fmt, ap); @@ -70,6 +85,7 @@ int qvsnprintf(char *str, size_t n, const char *fmt, va_list ap) #endif /*! + \fn int qsnprintf(char *str, size_t n, const char *fmt, ...) \target bytearray-qsnprintf \relates QByteArray @@ -89,6 +105,7 @@ int qvsnprintf(char *str, size_t n, const char *fmt, va_list ap) \sa qvsnprintf(), QString::asprintf() */ +Q_CORE_EXPORT // QT_NO_QSNPRINTF is in effect int qsnprintf(char *str, size_t n, const char *fmt, ...) { va_list ap; diff --git a/src/tools/bootstrap/CMakeLists.txt b/src/tools/bootstrap/CMakeLists.txt index ad95d77d00e..a1f8480a1d8 100644 --- a/src/tools/bootstrap/CMakeLists.txt +++ b/src/tools/bootstrap/CMakeLists.txt @@ -61,7 +61,6 @@ qt_internal_extend_target(Bootstrap ../../corelib/text/qstringbuilder.cpp ../../corelib/text/qstringconverter.cpp ../../corelib/text/qstringlist.cpp - ../../corelib/text/qvsnprintf.cpp ../../corelib/time/qcalendar.cpp ../../corelib/time/qdatetime.cpp ../../corelib/time/qgregoriancalendar.cpp