From 39e156e639ac4eadd7a0d4dac73d05db077e817b 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.7 6.6 6.5 Change-Id: I6979d02a7395405cbf23fffd17c8f03baf0ec00d Reviewed-by: Oswald Buddenhagen Reviewed-by: Thiago Macieira --- 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 bad11b1479..a5cc979635 100644 --- a/src/corelib/io/qresource.cpp +++ b/src/corelib/io/qresource.cpp @@ -1349,6 +1349,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; @@ -1565,7 +1566,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"); @@ -1584,6 +1584,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; } @@ -1601,6 +1607,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 f5e8c849a6..56972ea764 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 fc9b9ee201..acbb8a6f68 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")); QVERIFY(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()); }