From 2948ab16ea45add3d5016684f1e97260f9f92f55 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 6 Jun 2023 16:33:21 +0200 Subject: [PATCH] QPixmapCache: deprecate replace() The replace() implementation overwrites the passed Key key with a new version, const_cast'ing away the const from the key passed by reference-to-const. This is UB if the Key was originally declared const. Deprecate the function. Also inline the const_cast, so compilers can readily detect the UB even if users don't enable deprecation warnings. Due to the severity of the issue (UB), immediate deprecation is warranted. There appear to be no in-tree user of the API outside of tst_qpixmapcache.cpp. [ChangeLog][Deprecation Notice][QtGui][QPixmapCache] The `replace(key, pixmap)` function has been deprecated, because passing a `const Key` to it results in undefined behavior. Use `remove(key, pixmap)` followed by `key = insert(pixmap)` instead. Change-Id: Ic5060ce3271f2a1b6dc561da8716b452a2355d4c Reviewed-by: Volker Hilsheimer (cherry picked from commit 78cdd9a64dc0cd666e5c8daafa7477c29641420d) --- src/gui/compat/removed_api.cpp | 2 + src/gui/image/qpixmapcache.cpp | 39 +++++-------------- src/gui/image/qpixmapcache.h | 17 ++++++++ .../image/qpixmapcache/tst_qpixmapcache.cpp | 11 ++++++ 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/gui/compat/removed_api.cpp b/src/gui/compat/removed_api.cpp index c38b30d812e..8d1c8a19209 100644 --- a/src/gui/compat/removed_api.cpp +++ b/src/gui/compat/removed_api.cpp @@ -9,6 +9,8 @@ QT_USE_NAMESPACE #if QT_GUI_REMOVED_SINCE(6, 6) +#include "qpixmapcache.h" // inlined API + // #include "qotherheader.h" // // implement removed functions from qotherheader.h // order sections alphabetically diff --git a/src/gui/image/qpixmapcache.cpp b/src/gui/image/qpixmapcache.cpp index 09cc0b3753e..76fba1eb936 100644 --- a/src/gui/image/qpixmapcache.cpp +++ b/src/gui/image/qpixmapcache.cpp @@ -184,7 +184,6 @@ public: void timerEvent(QTimerEvent *) override; bool insert(const QString& key, const QPixmap &pixmap, int cost); QPixmapCache::Key insert(const QPixmap &pixmap, int cost); - bool replace(const QPixmapCache::Key &key, const QPixmap &pixmap, int cost); bool remove(const QString &key); bool remove(const QPixmapCache::Key &key); @@ -351,25 +350,6 @@ QPixmapCache::Key QPMCache::insert(const QPixmap &pixmap, int cost) return cacheKey; } -bool QPMCache::replace(const QPixmapCache::Key &key, const QPixmap &pixmap, int cost) -{ - Q_ASSERT(key.isValid()); - //If for the same key we had already an entry so we should delete the pixmap and use the new one - QCache::remove(key); - - QPixmapCache::Key cacheKey = createKey(); - - bool success = QCache::insert(cacheKey, new QPixmapCacheEntry(cacheKey, pixmap), cost); - if (success) { - if (!theid) { - theid = startTimer(flush_time); - t = false; - } - const_cast(key) = cacheKey; - } - return success; -} - bool QPMCache::remove(const QString &key) { auto cacheKey = cacheKeys.constFind(key); @@ -553,24 +533,25 @@ QPixmapCache::Key QPixmapCache::insert(const QPixmap &pixmap) return pm_cache()->insert(pixmap, cost(pixmap)); } +#if QT_DEPRECATED_SINCE(6, 6) /*! + \fn bool QPixmapCache::replace(const Key &key, const QPixmap &pixmap) + + \deprecated [6.6] Use \c{remove(key); key = insert(pixmap);} instead. + Replaces the pixmap associated with the given \a key with the \a pixmap specified. Returns \c true if the \a pixmap has been correctly inserted into the cache; otherwise returns \c false. + The passed \a key is updated to reference \a pixmap now. Other copies of \a + key, if any, still refer to the old pixmap, which is, however, removed from + the cache by this function. + \sa setCacheLimit(), insert() \since 4.6 */ -bool QPixmapCache::replace(const Key &key, const QPixmap &pixmap) -{ - if (!qt_pixmapcache_thread_test()) - return false; - //The key is not valid anymore, a flush happened before probably - if (!key.d || !key.d->isValid) - return false; - return pm_cache()->replace(key, pixmap, cost(pixmap)); -} +#endif // QT_DEPRECATED_SINCE(6, 6) /*! Returns the cache limit (in kilobytes). diff --git a/src/gui/image/qpixmapcache.h b/src/gui/image/qpixmapcache.h index 433890c68ff..55aad6503f3 100644 --- a/src/gui/image/qpixmapcache.h +++ b/src/gui/image/qpixmapcache.h @@ -42,13 +42,30 @@ public: static bool find(const Key &key, QPixmap *pixmap); static bool insert(const QString &key, const QPixmap &pixmap); static Key insert(const QPixmap &pixmap); +#if QT_DEPRECATED_SINCE(6, 6) + QT_DEPRECATED_VERSION_X_6_6("Use remove(key), followed by key = insert(pixmap).") + QT_GUI_INLINE_SINCE(6, 6) static bool replace(const Key &key, const QPixmap &pixmap); +#endif static void remove(const QString &key); static void remove(const Key &key); static void clear(); }; Q_DECLARE_SHARED(QPixmapCache::Key) +#if QT_DEPRECATED_SINCE(6, 6) +#if QT_GUI_INLINE_IMPL_SINCE(6, 6) +bool QPixmapCache::replace(const Key &key, const QPixmap &pixmap) +{ + if (!key.isValid()) + return false; + remove(key); + const_cast(key) = insert(pixmap); + return key.isValid(); +} +#endif // QT_GUI_INLINE_IMPL_SINCE(6, 6) +#endif // QT_DEPRECATED_SINCE(6, 6) + QT_END_NAMESPACE #endif // QPIXMAPCACHE_H diff --git a/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp b/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp index b215e127cea..27aac5e5176 100644 --- a/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp +++ b/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp @@ -32,7 +32,9 @@ private slots: void find(); void insert(); void failedInsertReturnsInvalidKey(); +#if QT_DEPRECATED_SINCE(6, 6) void replace(); +#endif void remove(); void clear(); void pixmapKey(); @@ -113,11 +115,15 @@ void tst_QPixmapCache::setCacheLimit() QVERIFY(!QPixmapCache::find(key, p1)); QPixmapCache::setCacheLimit(1000); +#if QT_DEPRECATED_SINCE(6, 6) + QT_WARNING_PUSH + QT_WARNING_DISABLE_DEPRECATED p1 = new QPixmap(2, 3); QVERIFY(!QPixmapCache::replace(key, *p1)); QVERIFY(!QPixmapCache::find(key, p1)); delete p1; +#endif // QT_DEPRECATED_SINCE(6, 6) //Let check if keys are released when the pixmap cache is //full or has been flushed. @@ -310,6 +316,9 @@ void tst_QPixmapCache::failedInsertReturnsInvalidKey() QVERIFY(!success); // QString API } +#if QT_DEPRECATED_SINCE(6, 6) +QT_WARNING_PUSH +QT_WARNING_DISABLE_DEPRECATED void tst_QPixmapCache::replace() { //The int part of the API @@ -338,6 +347,8 @@ void tst_QPixmapCache::replace() //Broken keys QCOMPARE(QPixmapCache::replace(QPixmapCache::Key(), p2), false); } +QT_WARNING_POP +#endif // QT_DEPRECATED_SINCE(6, 6) void tst_QPixmapCache::remove() {