Skip to content

Commit

Permalink
Fix crash. Remove unnecessary dependency injection causing crash.
Browse files Browse the repository at this point in the history
Signed-off-by: alex-z <[email protected]>
  • Loading branch information
allexzander authored and mgallien committed Nov 9, 2023
1 parent ab3e4e7 commit dd178f0
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 66 deletions.
4 changes: 2 additions & 2 deletions src/common/checksumcalculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ static QCryptographicHash::Algorithm algorithmTypeToQCryptoHashAlgorithm(Checksu
return static_cast<QCryptographicHash::Algorithm>(-1);
}

ChecksumCalculator::ChecksumCalculator(QSharedPointer<QIODevice> sharedDevice, const QByteArray &checksumTypeName)
: _device(sharedDevice)
ChecksumCalculator::ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName)
: _device(new QFile(filePath))
{
if (checksumTypeName == checkSumMD5C) {
_algorithmType = AlgorithmType::MD5;
Expand Down
7 changes: 3 additions & 4 deletions src/common/checksumcalculator.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
#include <QByteArray>
#include <QFutureWatcher>
#include <QMutex>

#include <memory>
#include <QScopedPointer>

class QCryptographicHash;

Expand All @@ -42,14 +41,14 @@ class OCSYNC_EXPORT ChecksumCalculator
Adler32,
};

ChecksumCalculator(QSharedPointer<QIODevice> 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<QIODevice> _device;
QScopedPointer<QIODevice> _device;
QScopedPointer<QCryptographicHash> _cryptographicHash;
unsigned int _adlerHash = 0;
bool _isInitialized = false;
Expand Down
28 changes: 6 additions & 22 deletions src/common/checksums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,43 +187,34 @@ QByteArray ComputeChecksum::checksumType() const
void ComputeChecksum::start(const QString &filePath)
{
qCInfo(lcChecksums) << "Computing" << checksumType() << "checksum of" << filePath << "in a thread";
startImpl(QSharedPointer<QFile>::create(filePath));
startImpl(filePath);
}

void ComputeChecksum::start(QSharedPointer<QIODevice> device)
{
ENFORCE(device);
qCInfo(lcChecksums) << "Computing" << checksumType() << "checksum of device" << device.get() << "in a thread";
ASSERT(!device->parent());

startImpl(device);
}

void ComputeChecksum::startImpl(QSharedPointer<QIODevice> 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();
}));
}

QByteArray ComputeChecksum::computeNowOnFile(const QString &filePath, const QByteArray &checksumType)
{
return computeNow(QSharedPointer<QFile>::create(filePath), checksumType);
return computeNow(filePath, checksumType);
}

QByteArray ComputeChecksum::computeNow(QSharedPointer<QIODevice> 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();
}

Expand Down Expand Up @@ -270,13 +261,6 @@ void ValidateChecksumHeader::start(const QString &filePath, const QByteArray &ch
calculator->start(filePath);
}

void ValidateChecksumHeader::start(QSharedPointer<QIODevice> device, const QByteArray &checksumHeader)
{
if (auto calculator = prepareStart(checksumHeader)) {
calculator->start(device);
}
}

QByteArray ValidateChecksumHeader::calculatedChecksumType() const
{
return _calculatedChecksumType;
Expand Down
24 changes: 2 additions & 22 deletions src/common/checksums.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<QIODevice> device);

/**
* Computes the checksum synchronously.
*/
static QByteArray computeNow(QSharedPointer<QIODevice> device, const QByteArray &checksumType);
static QByteArray computeNow(const QString &filePath, const QByteArray &checksumType);

/**
* Computes the checksum synchronously on file. Convenience wrapper for computeNow().
Expand All @@ -108,7 +98,7 @@ private slots:
void slotCalculationDone();

private:
void startImpl(QSharedPointer<QIODevice> device);
void startImpl(const QString &filePath);

QByteArray _checksumType;

Expand Down Expand Up @@ -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<QIODevice> device, const QByteArray &checksumHeader);

[[nodiscard]] QByteArray calculatedChecksumType() const;
[[nodiscard]] QByteArray calculatedChecksum() const;

Expand Down
22 changes: 6 additions & 16 deletions test/testchecksumvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ using namespace OCC::Utility;
QFileInfo fi(file);
QVERIFY(fi.exists());

auto sharedFile(QSharedPointer<QFile>::create(file));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumMD5C);
ChecksumCalculator checksumCalculator(file, OCC::checkSumMD5C);

const auto sum = checksumCalculator.calculate();

Expand All @@ -106,8 +105,7 @@ using namespace OCC::Utility;
QFileInfo fi(file);
QVERIFY(fi.exists());

auto sharedFile(QSharedPointer<QFile>::create(file));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumSHA1C);
ChecksumCalculator checksumCalculator(file, OCC::checkSumSHA1C);

const auto sum = checksumCalculator.calculate();

Expand All @@ -129,8 +127,7 @@ using namespace OCC::Utility;

connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);

auto sharedFile(QSharedPointer<QFile>::create(_testfile));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumAdlerC);
ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumAdlerC);

_expected = checksumCalculator.calculate();

Expand All @@ -152,8 +149,7 @@ using namespace OCC::Utility;
vali->setChecksumType(_expectedType);
connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);

auto sharedFile(QSharedPointer<QFile>::create(_testfile));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumMD5C);
ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumMD5C);

_expected = checksumCalculator.calculate();
vali->start(_testfile);
Expand All @@ -172,8 +168,7 @@ using namespace OCC::Utility;
vali->setChecksumType(_expectedType);
connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);

auto sharedFile(QSharedPointer<QFile>::create(_testfile));
ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumSHA1C);
ChecksumCalculator checksumCalculator(_testfile, OCC::checkSumSHA1C);
_expected = checksumCalculator.calculate();

vali->start(_testfile);
Expand All @@ -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<QFile>::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);

Expand All @@ -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);

Expand Down

0 comments on commit dd178f0

Please sign in to comment.