From dd178f032c704b01a33ed1e22abccbf337730332 Mon Sep 17 00:00:00 2001 From: alex-z Date: Fri, 27 Oct 2023 20:32:40 +0200 Subject: [PATCH] Fix crash. Remove unnecessary dependency injection causing crash. Signed-off-by: alex-z --- src/common/checksumcalculator.cpp | 4 ++-- src/common/checksumcalculator.h | 7 +++---- src/common/checksums.cpp | 28 ++++++---------------------- src/common/checksums.h | 24 ++---------------------- test/testchecksumvalidator.cpp | 22 ++++++---------------- 5 files changed, 19 insertions(+), 66 deletions(-) diff --git a/src/common/checksumcalculator.cpp b/src/common/checksumcalculator.cpp index 016c7b09d9b1b..ec9faa2de8c5a 100644 --- a/src/common/checksumcalculator.cpp +++ b/src/common/checksumcalculator.cpp @@ -47,8 +47,8 @@ static QCryptographicHash::Algorithm algorithmTypeToQCryptoHashAlgorithm(Checksu return static_cast(-1); } -ChecksumCalculator::ChecksumCalculator(QSharedPointer sharedDevice, const QByteArray &checksumTypeName) - : _device(sharedDevice) +ChecksumCalculator::ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName) + : _device(new QFile(filePath)) { if (checksumTypeName == checkSumMD5C) { _algorithmType = AlgorithmType::MD5; diff --git a/src/common/checksumcalculator.h b/src/common/checksumcalculator.h index c9c90bbf3d21e..da1263abeee57 100644 --- a/src/common/checksumcalculator.h +++ b/src/common/checksumcalculator.h @@ -22,8 +22,7 @@ #include #include #include - -#include +#include class QCryptographicHash; @@ -42,14 +41,14 @@ class OCSYNC_EXPORT ChecksumCalculator Adler32, }; - ChecksumCalculator(QSharedPointer sharedDevice, const QByteArray &checksumTypeName); + ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName); ~ChecksumCalculator(); [[nodiscard]] QByteArray calculate(); private: void initChecksumAlgorithm(); bool addChunk(const QByteArray &chunk, const qint64 size); - QSharedPointer _device; + QScopedPointer _device; QScopedPointer _cryptographicHash; unsigned int _adlerHash = 0; bool _isInitialized = false; diff --git a/src/common/checksums.cpp b/src/common/checksums.cpp index 9bc70b41b60a6..370730c3c3461 100644 --- a/src/common/checksums.cpp +++ b/src/common/checksums.cpp @@ -187,25 +187,16 @@ QByteArray ComputeChecksum::checksumType() const void ComputeChecksum::start(const QString &filePath) { qCInfo(lcChecksums) << "Computing" << checksumType() << "checksum of" << filePath << "in a thread"; - startImpl(QSharedPointer::create(filePath)); + startImpl(filePath); } -void ComputeChecksum::start(QSharedPointer device) -{ - ENFORCE(device); - qCInfo(lcChecksums) << "Computing" << checksumType() << "checksum of device" << device.get() << "in a thread"; - ASSERT(!device->parent()); - - startImpl(device); -} - -void ComputeChecksum::startImpl(QSharedPointer device) +void ComputeChecksum::startImpl(const QString &filePath) { connect(&_watcher, &QFutureWatcherBase::finished, this, &ComputeChecksum::slotCalculationDone, Qt::UniqueConnection); - _checksumCalculator.reset(new ChecksumCalculator(device, _checksumType)); + _checksumCalculator.reset(new ChecksumCalculator(filePath, _checksumType)); _watcher.setFuture(QtConcurrent::run([this]() { return _checksumCalculator->calculate(); })); @@ -213,17 +204,17 @@ void ComputeChecksum::startImpl(QSharedPointer device) QByteArray ComputeChecksum::computeNowOnFile(const QString &filePath, const QByteArray &checksumType) { - return computeNow(QSharedPointer::create(filePath), checksumType); + return computeNow(filePath, checksumType); } -QByteArray ComputeChecksum::computeNow(QSharedPointer device, const QByteArray &checksumType) +QByteArray ComputeChecksum::computeNow(const QString &filePath, const QByteArray &checksumType) { if (!checksumComputationEnabled()) { qCWarning(lcChecksums) << "Checksum computation disabled by environment variable"; return QByteArray(); } - ChecksumCalculator checksumCalculator(device, checksumType); + ChecksumCalculator checksumCalculator(filePath, checksumType); return checksumCalculator.calculate(); } @@ -270,13 +261,6 @@ void ValidateChecksumHeader::start(const QString &filePath, const QByteArray &ch calculator->start(filePath); } -void ValidateChecksumHeader::start(QSharedPointer device, const QByteArray &checksumHeader) -{ - if (auto calculator = prepareStart(checksumHeader)) { - calculator->start(device); - } -} - QByteArray ValidateChecksumHeader::calculatedChecksumType() const { return _calculatedChecksumType; diff --git a/src/common/checksums.h b/src/common/checksums.h index 9496c29edc168..bd13dc07fb82e 100644 --- a/src/common/checksums.h +++ b/src/common/checksums.h @@ -81,20 +81,10 @@ class OCSYNC_EXPORT ComputeChecksum : public QObject */ void start(const QString &filePath); - /** - * Computes the checksum for the given device. - * - * done() is emitted when the calculation finishes. - * - * The device ownership transfers into the thread that - * will compute the checksum. It must not have a parent. - */ - void start(QSharedPointer device); - /** * Computes the checksum synchronously. */ - static QByteArray computeNow(QSharedPointer device, const QByteArray &checksumType); + static QByteArray computeNow(const QString &filePath, const QByteArray &checksumType); /** * Computes the checksum synchronously on file. Convenience wrapper for computeNow(). @@ -108,7 +98,7 @@ private slots: void slotCalculationDone(); private: - void startImpl(QSharedPointer device); + void startImpl(const QString &filePath); QByteArray _checksumType; @@ -145,16 +135,6 @@ class OCSYNC_EXPORT ValidateChecksumHeader : public QObject */ void start(const QString &filePath, const QByteArray &checksumHeader); - /** - * Check a device's actual checksum against the provided checksumHeader - * - * Like the other start() but works on an device. - * - * The device ownership transfers into the thread that - * will compute the checksum. It must not have a parent. - */ - void start(QSharedPointer device, const QByteArray &checksumHeader); - [[nodiscard]] QByteArray calculatedChecksumType() const; [[nodiscard]] QByteArray calculatedChecksum() const; diff --git a/test/testchecksumvalidator.cpp b/test/testchecksumvalidator.cpp index e0684c8d7423c..313c03fad4e13 100644 --- a/test/testchecksumvalidator.cpp +++ b/test/testchecksumvalidator.cpp @@ -86,8 +86,7 @@ using namespace OCC::Utility; QFileInfo fi(file); QVERIFY(fi.exists()); - auto sharedFile(QSharedPointer::create(file)); - ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumMD5C); + ChecksumCalculator checksumCalculator(file, OCC::checkSumMD5C); const auto sum = checksumCalculator.calculate(); @@ -106,8 +105,7 @@ using namespace OCC::Utility; QFileInfo fi(file); QVERIFY(fi.exists()); - auto sharedFile(QSharedPointer::create(file)); - ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumSHA1C); + ChecksumCalculator checksumCalculator(file, OCC::checkSumSHA1C); const auto sum = checksumCalculator.calculate(); @@ -129,8 +127,7 @@ using namespace OCC::Utility; connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated); - auto sharedFile(QSharedPointer::create(_testfile)); - ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumAdlerC); + ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumAdlerC); _expected = checksumCalculator.calculate(); @@ -152,8 +149,7 @@ using namespace OCC::Utility; vali->setChecksumType(_expectedType); connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated); - auto sharedFile(QSharedPointer::create(_testfile)); - ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumMD5C); + ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumMD5C); _expected = checksumCalculator.calculate(); vali->start(_testfile); @@ -172,8 +168,7 @@ using namespace OCC::Utility; vali->setChecksumType(_expectedType); connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated); - auto sharedFile(QSharedPointer::create(_testfile)); - ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumSHA1C); + ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumSHA1C); _expected = checksumCalculator.calculate(); vali->start(_testfile); @@ -193,16 +188,13 @@ using namespace OCC::Utility; connect(vali, &ValidateChecksumHeader::validated, this, &TestChecksumValidator::slotDownValidated); connect(vali, &ValidateChecksumHeader::validationFailed, this, &TestChecksumValidator::slotDownError); - auto sharedFile(QSharedPointer::create(_testfile)); - ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumAdlerC); + ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumAdlerC); _expected = checksumCalculator.calculate(); QByteArray adler = checkSumAdlerC; adler.append(":"); adler.append(_expected); - sharedFile->open(QIODevice::ReadOnly); - sharedFile->seek(0); _successDown = false; vali->start(_testfile, adler); @@ -211,14 +203,12 @@ using namespace OCC::Utility; _expectedError = QStringLiteral("The downloaded file does not match the checksum, it will be resumed. \"543345\" != \"%1\"").arg(QString::fromUtf8(_expected)); _expectedFailureReason = ValidateChecksumHeader::FailureReason::ChecksumMismatch; _errorSeen = false; - sharedFile->seek(0); vali->start(_testfile, "Adler32:543345"); QTRY_VERIFY(_errorSeen); _expectedError = QLatin1String("The checksum header contained an unknown checksum type \"Klaas32\""); _expectedFailureReason = ValidateChecksumHeader::FailureReason::ChecksumTypeUnknown; _errorSeen = false; - sharedFile->seek(0); vali->start(_testfile, "Klaas32:543345"); QTRY_VERIFY(_errorSeen);