Fix review comments.

Signed-off-by: alex-z <[email protected]>
nextcloud · Jun 20, 2023 · 089cda6 · 089cda6
1 parent 52e9985
commit 089cda6
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 19 deletions.
diff --git a/src/common/checksumcalculator.cpp b/src/common/checksumcalculator.cpp
@@ -12,23 +12,28 @@
  * for more details.
  */
 #include "checksumcalculator.h"
-#include <QCryptographicHash>
-#include <QFile>
 
 #include <zlib.h>
 
+#include <QCryptographicHash>
+#include <QFile>
+#include <QLoggingCategory>
+
 namespace
 {
 constexpr qint64 bufSize = 500 * 1024;
 }
 
 namespace OCC {
 
+Q_LOGGING_CATEGORY(lcChecksumCalculator, "nextcloud.common.checksumcalculator", QtInfoMsg)
+
 static QCryptographicHash::Algorithm algorithmTypeToQCryptoHashAlgorithm(ChecksumCalculator::AlgorithmType algorithmType)
 {
     switch (algorithmType) {
     case ChecksumCalculator::AlgorithmType::Undefined:
     case ChecksumCalculator::AlgorithmType::Adler32:
+        qCWarning(lcChecksumCalculator) << "Invalid algorithm type" << static_cast<int>(algorithmType);
         return static_cast<QCryptographicHash::Algorithm>(-1);
     case ChecksumCalculator::AlgorithmType::MD5:
         return QCryptographicHash::Algorithm::Md5;
@@ -78,14 +83,14 @@ QByteArray ChecksumCalculator::calculate()
 
     Q_ASSERT(!_device->isOpen());
     if (_device->isOpen()) {
-        qWarning() << "Device already open. Ignoring.";
+        qCWarning(lcChecksumCalculator) << "Device already open. Ignoring.";
     }
 
     if (!_device->isOpen() && !_device->open(QIODevice::ReadOnly)) {
         if (auto file = qobject_cast<QFile *>(_device.data())) {
-            qWarning() << "Could not open file" << file->fileName() << "for reading to compute a checksum" << file->errorString();
+            qCWarning(lcChecksumCalculator) << "Could not open file" << file->fileName() << "for reading to compute a checksum" << file->errorString();
         } else {
-            qWarning() << "Could not open device" << _device.data() << "for reading to compute a checksum" << _device->errorString();
+            qCWarning(lcChecksumCalculator) << "Could not open device" << _device.data() << "for reading to compute a checksum" << _device->errorString();
         }
         return result;
     }
@@ -143,6 +148,7 @@ QByteArray ChecksumCalculator::calculate()
 void ChecksumCalculator::initChecksumAlgorithm()
 {
     if (_algorithmType == AlgorithmType::Undefined) {
+        qCWarning(lcChecksumCalculator) << "_algorithmType is Undefined, impossible to init Checksum Algorithm";
         return;
     }
 
@@ -166,6 +172,7 @@ bool ChecksumCalculator::addChunk(const QByteArray &chunk, const qint64 size)
 {
     Q_ASSERT(_algorithmType != AlgorithmType::Undefined);
     if (_algorithmType == AlgorithmType::Undefined) {
+        qCWarning(lcChecksumCalculator) << "_algorithmType is Undefined, impossible to add a chunk!";
         return false;
     }
 

diff --git a/src/common/checksumcalculator.h b/src/common/checksumcalculator.h
@@ -44,7 +44,7 @@ class OCSYNC_EXPORT ChecksumCalculator
 
     ChecksumCalculator(QSharedPointer<QIODevice> sharedDevice, const QByteArray &checksumTypeName);
     ~ChecksumCalculator();
-    QByteArray calculate();
+    [[nodiscard]] QByteArray calculate();
 
 private:
     void initChecksumAlgorithm();

diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp
@@ -76,7 +76,6 @@ void DiskFileModifier::setContents(const QString &relativePath, char contentChar
     qint64 size = file.size();
     file.open(QFile::WriteOnly);
     file.write(QByteArray {}.fill(contentChar, size));
-    file.close();
 }
 
 void DiskFileModifier::appendByte(const QString &relativePath)
@@ -87,7 +86,6 @@ void DiskFileModifier::appendByte(const QString &relativePath)
     QByteArray contents = file.read(1);
     file.seek(file.size());
     file.write(contents);
-    file.close();
 }
 
 void DiskFileModifier::mkdir(const QString &relativePath)

diff --git a/test/testchecksumvalidator.cpp b/test/testchecksumvalidator.cpp
@@ -86,8 +86,7 @@ using namespace OCC::Utility;
         QFileInfo fi(file);
         QVERIFY(fi.exists());
 
-        QSharedPointer<QFile> fileDevice(new QFile(file));
-        const auto sharedFile = QSharedPointer<QIODevice>(fileDevice);
+        auto sharedFile(QSharedPointer<QFile>::create(file));
         ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumMD5C);
 
         const auto sum = checksumCalculator.calculate();
@@ -107,8 +106,7 @@ using namespace OCC::Utility;
         QFileInfo fi(file);
         QVERIFY(fi.exists());
 
-        QSharedPointer<QFile> fileDevice(new QFile(file));
-        const auto sharedFile = QSharedPointer<QIODevice>(fileDevice);
+        auto sharedFile(QSharedPointer<QFile>::create(file));
         ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumSHA1C);
 
         const auto sum = checksumCalculator.calculate();
@@ -131,8 +129,7 @@ using namespace OCC::Utility;
 
         connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);
 
-        QSharedPointer<QFile> file(new QFile(_testfile));
-        const auto sharedFile = QSharedPointer<QIODevice>(file);
+        auto sharedFile(QSharedPointer<QFile>::create(_testfile));
         ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumAdlerC);
 
         _expected = checksumCalculator.calculate();
@@ -155,8 +152,7 @@ using namespace OCC::Utility;
         vali->setChecksumType(_expectedType);
         connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);
 
-        QSharedPointer<QFile> file(new QFile(_testfile));
-        const auto sharedFile = QSharedPointer<QIODevice>(file);
+        auto sharedFile(QSharedPointer<QFile>::create(_testfile));
         ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumMD5C);
 
         _expected = checksumCalculator.calculate();
@@ -176,8 +172,7 @@ using namespace OCC::Utility;
         vali->setChecksumType(_expectedType);
         connect(vali, &ComputeChecksum::done, this, &TestChecksumValidator::slotUpValidated);
 
-        QSharedPointer<QFile> file(new QFile(_testfile));
-        const auto sharedFile = QSharedPointer<QIODevice>(file);
+        auto sharedFile(QSharedPointer<QFile>::create(_testfile));
         ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumSHA1C);
         _expected = checksumCalculator.calculate();
 
@@ -198,7 +193,7 @@ using namespace OCC::Utility;
         connect(vali, &ValidateChecksumHeader::validated, this, &TestChecksumValidator::slotDownValidated);
         connect(vali, &ValidateChecksumHeader::validationFailed, this, &TestChecksumValidator::slotDownError);
 
-        QSharedPointer<QFile> sharedFile(new QFile(_testfile));
+        auto sharedFile(QSharedPointer<QFile>::create(_testfile));
         ChecksumCalculator checksumCalculator(sharedFile, OCC::checkSumAdlerC);
         _expected = checksumCalculator.calculate();