Merge pull request #15419 from Chocobo1/file

Improve error detection when saving files

src/app/upgrade.cpp

@@ -28,7 +28,6 @@
 
 #include "upgrade.h"
 
-#include <QFile>
 #include <QMetaEnum>
 #include <QVector>
 
@@ -37,6 +36,7 @@
 #include "base/profile.h"
 #include "base/settingsstorage.h"
 #include "base/utils/fs.h"
+#include "base/utils/io.h"
 #include "base/utils/string.h"
 
 namespace
@@ -53,17 +53,10 @@ namespace
             if (!newData.isEmpty() || oldData.isEmpty())
                 return;
 
-            QFile file(savePath);
+            const nonstd::expected<void, QString> result = Utils::IO::saveToFile(savePath, oldData);
-            if (!file.open(QIODevice::WriteOnly))
+            if (!result)
             {
-                LogMsg(errorMsgFormat.arg(savePath, file.errorString()) , Log::WARNING);
+                LogMsg(errorMsgFormat.arg(savePath, result.error()) , Log::WARNING);
-                return;
-            }
-            if (file.write(oldData) != oldData.size())
-            {
-                file.close();
-                Utils::Fs::forceRemove(savePath);
-                LogMsg(errorMsgFormat.arg(savePath, QLatin1String("Write incomplete.")) , Log::WARNING);
                 return;
             }
 

src/base/CMakeLists.txt

@@ -1,5 +1,6 @@
 add_library(qbt_base STATIC
     # headers
+    3rdparty/expected.hpp
     algorithm.h
     asyncfilestorage.h
     bittorrent/abstractfilestorage.h

src/base/asyncfilestorage.cpp

@@ -30,7 +30,8 @@
 
 #include <QDebug>
 #include <QMetaObject>
-#include <QSaveFile>
+
+#include "base/utils/io.h"
 
 AsyncFileStorage::AsyncFileStorage(const QString &storageFolderPath, QObject *parent)
     : QObject(parent)
@@ -67,15 +68,12 @@ QDir AsyncFileStorage::storageDir() const
 void AsyncFileStorage::store_impl(const QString &fileName, const QByteArray &data)
 {
     const QString filePath = m_storageDir.absoluteFilePath(fileName);
-    QSaveFile file(filePath);
     qDebug() << "AsyncFileStorage: Saving data to" << filePath;
-    if (file.open(QIODevice::WriteOnly))
+
-    {
+    const nonstd::expected<void, QString> result = Utils::IO::saveToFile(filePath, data);
-        file.write(data);
+    if (!result)
-        if (!file.commit())
     {
         qDebug() << "AsyncFileStorage: Failed to save data";
-            emit failed(filePath, file.errorString());
+        emit failed(filePath, result.error());
-        }
     }
 }

src/base/base.pri

@@ -1,4 +1,5 @@
 HEADERS += \
+    $$PWD/3rdparty/expected.hpp \
     $$PWD/algorithm.h \
     $$PWD/asyncfilestorage.h \
     $$PWD/bittorrent/abstractfilestorage.h \

src/base/bittorrent/bencoderesumedatastorage.cpp

@@ -29,7 +29,6 @@
 #include "bencoderesumedatastorage.h"
 
 #include <libtorrent/bdecode.hpp>
-#include <libtorrent/bencode.hpp>
 #include <libtorrent/entry.hpp>
 #include <libtorrent/read_resume_data.hpp>
 #include <libtorrent/torrent_info.hpp>
@@ -37,7 +36,6 @@
 
 #include <QByteArray>
 #include <QRegularExpression>
-#include <QSaveFile>
 #include <QThread>
 
 #include "base/algorithm.h"
@@ -88,17 +86,6 @@ namespace
             entryList.emplace_back(setValue.toStdString());
         return entryList;
     }
-
-    void writeEntryToFile(const QString &filepath, const lt::entry &data)
-    {
-        QSaveFile file {filepath};
-        if (!file.open(QIODevice::WriteOnly))
-            throw RuntimeError(file.errorString());
-
-        lt::bencode(Utils::IO::FileDeviceOutputIterator {file}, data);
-        if (file.error() != QFileDevice::NoError || !file.commit())
-            throw RuntimeError(file.errorString());
-    }
 }
 
 BitTorrent::BencodeResumeDataStorage::BencodeResumeDataStorage(const QString &path, QObject *parent)
@@ -355,14 +342,11 @@ void BitTorrent::BencodeResumeDataStorage::Worker::store(const TorrentID &id, co
         metadataDict.insert(dataDict.extract("comment"));
 
         const QString torrentFilepath = m_resumeDataDir.absoluteFilePath(QString::fromLatin1("%1.torrent").arg(id.toString()));
-        try
+        const nonstd::expected<void, QString> result = Utils::IO::saveToFile(torrentFilepath, metadata);
-        {
+        if (!result)
-            writeEntryToFile(torrentFilepath, metadata);
-        }
-        catch (const RuntimeError &err)
         {
             LogMsg(tr("Couldn't save torrent metadata to '%1'. Error: %2.")
-                   .arg(torrentFilepath, err.message()), Log::CRITICAL);
+                   .arg(torrentFilepath, result.error()), Log::CRITICAL);
             return;
         }
     }
@@ -378,14 +362,11 @@ void BitTorrent::BencodeResumeDataStorage::Worker::store(const TorrentID &id, co
     data["qBt-firstLastPiecePriority"] = resumeData.firstLastPiecePriority;
 
     const QString resumeFilepath = m_resumeDataDir.absoluteFilePath(QString::fromLatin1("%1.fastresume").arg(id.toString()));
-    try
+    const nonstd::expected<void, QString> result = Utils::IO::saveToFile(resumeFilepath, data);
-    {
+    if (!result)
-        writeEntryToFile(resumeFilepath, data);
-    }
-    catch (const RuntimeError &err)
     {
         LogMsg(tr("Couldn't save torrent resume data to '%1'. Error: %2.")
-               .arg(resumeFilepath, err.message()), Log::CRITICAL);
+               .arg(resumeFilepath, result.error()), Log::CRITICAL);
     }
 }
 
@@ -406,10 +387,10 @@ void BitTorrent::BencodeResumeDataStorage::Worker::storeQueue(const QVector<Torr
         data += (torrentID.toString().toLatin1() + '\n');
 
     const QString filepath = m_resumeDataDir.absoluteFilePath(QLatin1String("queue"));
-    QSaveFile file {filepath};
+    const nonstd::expected<void, QString> result = Utils::IO::saveToFile(filepath, data);
-    if (!file.open(QIODevice::WriteOnly) || (file.write(data) != data.size()) || !file.commit())
+    if (!result)
     {
         LogMsg(tr("Couldn't save data to '%1'. Error: %2")
-            .arg(filepath, file.errorString()), Log::CRITICAL);
+            .arg(filepath, result.error()), Log::CRITICAL);
     }
 }

src/base/bittorrent/torrentcreatorthread.cpp

@@ -30,13 +30,11 @@
 
 #include <fstream>
 
-#include <libtorrent/bencode.hpp>
 #include <libtorrent/create_torrent.hpp>
 #include <libtorrent/file_storage.hpp>
 #include <libtorrent/torrent_info.hpp>
 
 #include <QDirIterator>
-#include <QFile>
 #include <QFileInfo>
 #include <QHash>
 
@@ -209,16 +207,9 @@ void TorrentCreatorThread::run()
         checkInterruptionRequested();
 
         // create the torrent
-        QFile outfile {m_params.savePath};
+        const nonstd::expected<void, QString> result = Utils::IO::saveToFile(m_params.savePath, entry);
-        if (!outfile.open(QIODevice::WriteOnly))
+        if (!result)
-            throw RuntimeError(outfile.errorString());
+            throw RuntimeError(result.error());
-
-        checkInterruptionRequested();
-
-        lt::bencode(Utils::IO::FileDeviceOutputIterator {outfile}, entry);
-        if (outfile.error() != QFileDevice::NoError)
-            throw RuntimeError(outfile.errorString());
-        outfile.close();
 
         emit updateProgress(100);
         emit creationSuccess(m_params.savePath, parentPath);

src/base/bittorrent/torrentinfo.cpp

@@ -28,7 +28,6 @@
 
 #include "torrentinfo.h"
 
-#include <libtorrent/bencode.hpp>
 #include <libtorrent/create_torrent.hpp>
 #include <libtorrent/error_code.hpp>
 
@@ -175,14 +174,9 @@ void TorrentInfo::saveToFile(const QString &path) const
     {
         const auto torrentCreator = lt::create_torrent(*nativeInfo());
         const lt::entry torrentEntry = torrentCreator.generate();
-
+        const nonstd::expected<void, QString> result = Utils::IO::saveToFile(path, torrentEntry);
-        QFile torrentFile {path};
+        if (!result)
-        if (!torrentFile.open(QIODevice::WriteOnly))
+            throw RuntimeError(result.error());
-            throw RuntimeError(torrentFile.errorString());
-
-        lt::bencode(Utils::IO::FileDeviceOutputIterator {torrentFile}, torrentEntry);
-        if (torrentFile.error() != QFileDevice::NoError)
-            throw RuntimeError(torrentFile.errorString());
     }
     catch (const lt::system_error &err)
     {

src/base/net/downloadhandlerimpl.cpp

@@ -34,6 +34,7 @@
 
 #include "base/utils/fs.h"
 #include "base/utils/gzip.h"
+#include "base/utils/io.h"
 #include "base/utils/misc.h"
 
 const int MAX_REDIRECTIONS = 20;  // the common value for web browsers
@@ -43,24 +44,14 @@ namespace
     bool saveToFile(const QByteArray &replyData, QString &filePath)
     {
         if (!filePath.isEmpty())
-        {
+            return Utils::IO::saveToFile(filePath, replyData).has_value();
-            QFile file {filePath};
+
-            if (!file.open(QIODevice::WriteOnly))
+        QTemporaryFile file {Utils::Fs::tempPath()};
+        if (!file.open() || (file.write(replyData) != replyData.length()) || !file.flush())
             return false;
 
-            file.write(replyData);
+        file.setAutoRemove(false);
-            return true;
+        filePath = file.fileName();
-        }
-
-        QTemporaryFile tmpfile {Utils::Fs::tempPath() + "XXXXXX"};
-        tmpfile.setAutoRemove(false);
-
-        if (!tmpfile.open())
-            return false;
-
-        filePath = tmpfile.fileName();
-
-        tmpfile.write(replyData);
         return true;
     }
 }

src/base/net/geoipmanager.cpp

@@ -31,7 +31,6 @@
 
 #include <QDateTime>
 #include <QDir>
-#include <QFile>
 #include <QHostAddress>
 #include <QLocale>
 
@@ -40,6 +39,7 @@
 #include "base/profile.h"
 #include "base/utils/fs.h"
 #include "base/utils/gzip.h"
+#include "base/utils/io.h"
 #include "downloadmanager.h"
 #include "geoipdatabase.h"
 
@@ -451,13 +451,20 @@ void GeoIPManager::downloadFinished(const DownloadResult &result)
                         specialFolderLocation(SpecialFolder::Data) + GEODB_FOLDER);
             if (!QDir(targetPath).exists())
                 QDir().mkpath(targetPath);
-            QFile targetFile(QString::fromLatin1("%1/%2").arg(targetPath, GEODB_FILENAME));
+
-            if (!targetFile.open(QFile::WriteOnly) || (targetFile.write(data) == -1))
+            const auto path = QString::fromLatin1("%1/%2").arg(targetPath, GEODB_FILENAME);
-                LogMsg(tr("Couldn't save downloaded IP geolocation database file."), Log::WARNING);
+            const nonstd::expected<void, QString> result = Utils::IO::saveToFile(path, data);
-            else
+            if (result)
+            {
                 LogMsg(tr("Successfully updated IP geolocation database."), Log::INFO);
             }
             else
+            {
+                LogMsg(tr("Couldn't save downloaded IP geolocation database file. Reason: %1")
+                    .arg(result.error()), Log::WARNING);
+            }
+        }
+        else
         {
             delete geoIPDatabase;
         }

src/base/rss/rss_autodownloader.cpp

@@ -33,7 +33,6 @@
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonValue>
-#include <QSaveFile>
 #include <QThread>
 #include <QTimer>
 #include <QUrl>

src/base/rss/rss_session.cpp

@@ -34,7 +34,6 @@
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonValue>
-#include <QSaveFile>
 #include <QString>
 #include <QThread>
 

src/base/torrentfileswatcher.cpp

@@ -40,7 +40,6 @@
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonValue>
-#include <QSaveFile>
 #include <QSet>
 #include <QTextStream>
 #include <QThread>
@@ -60,6 +59,7 @@
 #include "base/settingsstorage.h"
 #include "base/tagset.h"
 #include "base/utils/fs.h"
+#include "base/utils/io.h"
 #include "base/utils/string.h"
 
 using namespace std::chrono_literals;
@@ -373,13 +373,13 @@ void TorrentFilesWatcher::store() const
         jsonObj[watchedFolder] = serializeWatchedFolderOptions(options);
     }
 
+    const QString path = QDir(specialFolderLocation(SpecialFolder::Config)).absoluteFilePath(CONF_FILE_NAME);
     const QByteArray data = QJsonDocument(jsonObj).toJson();
-
+    const nonstd::expected<void, QString> result = Utils::IO::saveToFile(path, data);
-    QSaveFile confFile {QDir(specialFolderLocation(SpecialFolder::Config)).absoluteFilePath(CONF_FILE_NAME)};
+    if (!result)
-    if (!confFile.open(QIODevice::WriteOnly) || (confFile.write(data) != data.size()) || !confFile.commit())
     {
         LogMsg(tr("Couldn't store Watched Folders configuration to %1. Error: %2")
-            .arg(confFile.fileName(), confFile.errorString()), Log::WARNING);
+            .arg(path, result.error()), Log::WARNING);
     }
 }
 

src/base/utils/io.cpp

@@ -28,8 +28,13 @@
 
 #include "io.h"
 
+#include <libtorrent/bencode.hpp>
+#include <libtorrent/entry.hpp>
+
 #include <QByteArray>
 #include <QFileDevice>
+#include <QSaveFile>
+#include <QString>
 
 Utils::IO::FileDeviceOutputIterator::FileDeviceOutputIterator(QFileDevice &device, const int bufferSize)
     : m_device {&device}
@@ -60,3 +65,24 @@ Utils::IO::FileDeviceOutputIterator &Utils::IO::FileDeviceOutputIterator::operat
     }
     return *this;
 }
+
+nonstd::expected<void, QString> Utils::IO::saveToFile(const QString &path, const QByteArray &data)
+{
+    QSaveFile file {path};
+    if (!file.open(QIODevice::WriteOnly) || (file.write(data) != data.size()) || !file.flush() || !file.commit())
+        return nonstd::make_unexpected(file.errorString());
+    return {};
+}
+
+nonstd::expected<void, QString> Utils::IO::saveToFile(const QString &path, const lt::entry &data)
+{
+    QSaveFile file {path};
+    if (!file.open(QIODevice::WriteOnly))
+        return nonstd::make_unexpected(file.errorString());
+
+    const int bencodedDataSize = lt::bencode(Utils::IO::FileDeviceOutputIterator {file}, data);
+    if ((file.size() != bencodedDataSize) || !file.flush() || !file.commit())
+        return nonstd::make_unexpected(file.errorString());
+
+    return {};
+}

src/base/utils/io.h

@@ -31,8 +31,13 @@
 #include <iterator>
 #include <memory>
 
+#include <libtorrent/fwd.hpp>
+
+#include "base/3rdparty/expected.hpp"
+
 class QByteArray;
 class QFileDevice;
+class QString;
 
 namespace Utils::IO
 {
@@ -74,4 +79,7 @@ namespace Utils::IO
         std::shared_ptr<QByteArray> m_buffer;
         int m_bufferSize;
     };
+
+    nonstd::expected<void, QString> saveToFile(const QString &path, const QByteArray &data);
+    nonstd::expected<void, QString> saveToFile(const QString &path, const lt::entry &data);
 }

src/gui/rss/automatedrssdownloader.cpp

@@ -49,6 +49,7 @@
 #include "base/rss/rss_session.h"
 #include "base/utils/compare.h"
 #include "base/utils/fs.h"
+#include "base/utils/io.h"
 #include "base/utils/string.h"
 #include "gui/autoexpandabledialog.h"
 #include "gui/torrentcategorydialog.h"
@@ -452,13 +453,12 @@ void AutomatedRssDownloader::on_exportBtn_clicked()
             path += EXT_LEGACY;
     }
 
-    QFile file {path};
+    const QByteArray rules = RSS::AutoDownloader::instance()->exportRules(format);
-    if (!file.open(QFile::WriteOnly)
+    const nonstd::expected<void, QString> result = Utils::IO::saveToFile(path, rules);
-            || (file.write(RSS::AutoDownloader::instance()->exportRules(format)) == -1))
+    if (!result)
     {
-        QMessageBox::critical(
+        QMessageBox::critical(this, tr("I/O Error")
-                    this, tr("I/O Error")
+            , tr("Failed to create the destination file. Reason: %1").arg(result.error()));
-                    , tr("Failed to create the destination file. Reason: %1").arg(file.errorString()));
     }
 }