From 1163efe559a22b4a971fac287eed6e3209f88576 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 23 Apr 2024 07:49:57 -0700 Subject: [PATCH] QResource: obey the MapPrivateOption option to provide RW memory The documentation says: The mapping will have the same open mode as the file (read and/or write), except when using MapPrivateOption, in which case it is always possible to write to the mapped memory. So obey it. This may cause high memory use by copying data we already have. This may be important because applications may want to memory-map resources which they intentionally didn't compress because those resources are large. Later commits will implement some workarounds. Fixes: QTBUG-124608 Pick-to: 6.6 6.5 Change-Id: I6979d02a7395405cbf23fffd17c8f03baf0ec00d Reviewed-by: Oswald Buddenhagen Reviewed-by: Thiago Macieira (cherry picked from commit 39e156e639ac4eadd7a0d4dac73d05db077e817b) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/io/qresource.cpp | 17 +++++++++++++- .../io/qresourceengine/testqrc/test.qrc | 6 ++--- .../qresourceengine/tst_qresourceengine.cpp | 23 ++++++++++++++++++- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/corelib/io/qresource.cpp b/src/corelib/io/qresource.cpp index 9f9cb0ab0bd..8c6a0fc6cef 100644 --- a/src/corelib/io/qresource.cpp +++ b/src/corelib/io/qresource.cpp @@ -1354,6 +1354,7 @@ private: uchar *map(qint64 offset, qint64 size, QFile::MemoryMapFlags flags); bool unmap(uchar *ptr); void uncompress() const; + void mapUncompressed(); qint64 offset; QResource resource; mutable QByteArray uncompressed; @@ -1577,7 +1578,6 @@ bool QResourceFileEngine::supportsExtension(Extension extension) const uchar *QResourceFileEnginePrivate::map(qint64 offset, qint64 size, QFile::MemoryMapFlags flags) { Q_Q(QResourceFileEngine); - Q_UNUSED(flags); Q_ASSERT_X(resource.compressionAlgorithm() == QResource::NoCompression || !uncompressed.isNull(), "QFile::map()", "open() should have uncompressed compressed resources"); @@ -1596,6 +1596,12 @@ uchar *QResourceFileEnginePrivate::map(qint64 offset, qint64 size, QFile::Memory // resource was not compressed address = resource.data(); + if (flags & QFile::MapPrivateOption) { + // We need to provide read-write memory + mapUncompressed(); + address = reinterpret_cast(uncompressed.constData()); + } + return const_cast(address) + offset; } @@ -1613,6 +1619,15 @@ void QResourceFileEnginePrivate::uncompress() const uncompressed = resource.uncompressedData(); } +void QResourceFileEnginePrivate::mapUncompressed() +{ + Q_ASSERT(resource.compressionAlgorithm() == QResource::NoCompression); + if (!uncompressed.isNull()) + return; // nothing to do + uncompressed = resource.uncompressedData(); + uncompressed.detach(); +} + #endif // !defined(QT_BOOTSTRAPPED) QT_END_NAMESPACE diff --git a/tests/auto/corelib/io/qresourceengine/testqrc/test.qrc b/tests/auto/corelib/io/qresourceengine/testqrc/test.qrc index f5e8c849a62..56972ea764a 100644 --- a/tests/auto/corelib/io/qresourceengine/testqrc/test.qrc +++ b/tests/auto/corelib/io/qresourceengine/testqrc/test.qrc @@ -9,10 +9,10 @@ searchpath1/search_file.txt searchpath2/search_file.txt search_file.txt - - test/testdir.txt + test/testdir.txt otherdir/otherdir.txt test/testdir2.txt + aliasdir/compressme.txt test/test @@ -21,7 +21,7 @@ aliasdir/compressme.txt - + test/german.txt diff --git a/tests/auto/corelib/io/qresourceengine/tst_qresourceengine.cpp b/tests/auto/corelib/io/qresourceengine/tst_qresourceengine.cpp index 8c536949994..f5fb73b5183 100644 --- a/tests/auto/corelib/io/qresourceengine/tst_qresourceengine.cpp +++ b/tests/auto/corelib/io/qresourceengine/tst_qresourceengine.cpp @@ -180,6 +180,7 @@ void tst_QResourceEngine::checkStructure_data() #if defined(BUILTIN_TESTDATA) << QLatin1String("testqrc") #endif + << QLatin1String("uncompresseddir") << QLatin1String("withoutslashes"); QTest::newRow("root dir") << QString(":/") @@ -384,8 +385,16 @@ void tst_QResourceEngine::checkStructure_data() QFile file(QFINDTESTDATA("testqrc/aliasdir/compressme.txt")); file.open(QFile::ReadOnly); info = QFileInfo(QFINDTESTDATA("testqrc/aliasdir/compressme.txt")); + QByteArray compressmeContents = file.readAll(); QTest::addRow("%s compressed text", qPrintable(root)) << QString(root + "aliasdir/aliasdir.txt") - << file.readAll() + << compressmeContents + << QStringList() + << QStringList() + << QLocale("de_CH") + << qlonglong(info.size()); + + QTest::addRow("%s non-compressed text", qPrintable(root)) << QString(root + "uncompresseddir/uncompressed.txt") + << compressmeContents << QStringList() << QStringList() << QLocale("de_CH") @@ -463,6 +472,18 @@ void tst_QResourceEngine::checkStructure() // check that it is still valid after closing the file file.close(); QCOMPARE(ba, contents); + + // memory should be writable because we used MapPrivateOption + *ptr = '\0'; + + // but shouldn't affect the actual file or a new mapping + QFile file2(pathName); + QVERIFY(file2.open(QFile::ReadOnly)); + QCOMPARE(file2.readAll(), contents); + ptr = file2.map(0, file.size(), QFile::MapPrivateOption); + QVERIFY2(ptr, qPrintable(file2.errorString())); + QByteArrayView bav(reinterpret_cast(ptr), file.size()); + QCOMPARE(bav, contents); } QLocale::setDefault(QLocale::system()); }