diff --git a/autotests/kcompressiondevicetest.h b/autotests/kcompressiondevicetest.h --- a/autotests/kcompressiondevicetest.h +++ b/autotests/kcompressiondevicetest.h @@ -54,6 +54,9 @@ void testGZipBufferedDevice(); void testBZip2BufferedDevice(); void testXzBufferedDevice(); + + void testWriteErrorOnOpen(); + void testWriteErrorOnClose(); }; #endif diff --git a/autotests/kcompressiondevicetest.cpp b/autotests/kcompressiondevicetest.cpp --- a/autotests/kcompressiondevicetest.cpp +++ b/autotests/kcompressiondevicetest.cpp @@ -91,6 +91,7 @@ void KCompressionDeviceTest::testExtraction() { QTemporaryDir temp; + QString oldCurrentDir = QDir::currentPath(); QDir::setCurrent(temp.path()); QVERIFY(archive->open(QIODevice::ReadOnly)); @@ -113,13 +114,14 @@ << QLatin1String("examples/unzipper/CMakeLists.txt") << QLatin1String("examples/unzipper/main.cpp"); - foreach (const QString s, fileList) { + foreach (const QString& s, fileList) { QFileInfo extractedFile(s); QFileInfo sourceFile(QFINDTESTDATA("../" + s)); QVERIFY(extractedFile.exists()); QCOMPARE(extractedFile.size(), sourceFile.size()); } + QDir::setCurrent(oldCurrentDir); } void KCompressionDeviceTest::regularKTarUsage() @@ -152,3 +154,35 @@ QSKIP("This test needs xz support"); #endif } + +void KCompressionDeviceTest::testWriteErrorOnOpen() +{ + // GIVEN + QString fileName("/I/dont/exist/kcompressiondevicetest-write.gz"); + KCompressionDevice dev(fileName, KCompressionDevice::GZip); + // WHEN + QVERIFY(!dev.open(QIODevice::WriteOnly)); + // THEN + QCOMPARE(dev.error(), QFileDevice::OpenError); + QCOMPARE(dev.errorString(), QStringLiteral("No such file or directory")); +} + +void KCompressionDeviceTest::testWriteErrorOnClose() +{ + // GIVEN + QFile file("kcompressiondevicetest-write.gz"); + KCompressionDevice dev(&file, false, KCompressionDevice::GZip); + QVERIFY(dev.open(QIODevice::WriteOnly)); + const QByteArray data = "Hello world"; + QCOMPARE(dev.write(data), data.size()); + // This is nasty, it's just a way to try and trigger an error on flush, without filling up a partition first ;) + file.close(); + QVERIFY(file.open(QIODevice::ReadOnly)); + QTest::ignoreMessage(QtWarningMsg, "QIODevice::write (QFile, \"kcompressiondevicetest-write.gz\"): ReadOnly device"); + + // WHEN + dev.close(); // I want a QVERIFY here... https://bugreports.qt.io/browse/QTBUG-70033 + + // THEN + QCOMPARE(int(dev.error()), int(QFileDevice::WriteError)); +} diff --git a/src/kcompressiondevice.h b/src/kcompressiondevice.h --- a/src/kcompressiondevice.h +++ b/src/kcompressiondevice.h @@ -21,6 +21,7 @@ #include #include +#include #include #include class KCompressionDevicePrivate; @@ -36,7 +37,7 @@ * Use this class to read/write compressed files. */ -class KARCHIVE_EXPORT KCompressionDevice : public QIODevice +class KARCHIVE_EXPORT KCompressionDevice : public QIODevice // KF6 TODO: consider inheriting from QFileDevice, so apps can use error() generically ? { public: enum CompressionType { @@ -113,14 +114,23 @@ */ static KFilterBase *filterForCompressionType(CompressionType type); + /** + * Returns the error code from the last failing operation. + * This is especially useful after calling close(), which unfortunately returns void + * (see https://bugreports.qt.io/browse/QTBUG-70033), to see if the flushing done by close + * was able to write all the data to disk. + */ + QFileDevice::FileError error() const; + protected: friend class K7Zip; qint64 readData(char *data, qint64 maxlen) override; qint64 writeData(const char *data, qint64 len) override; KFilterBase *filterBase(); private: + friend KCompressionDevicePrivate; KCompressionDevicePrivate *const d; }; diff --git a/src/kcompressiondevice.cpp b/src/kcompressiondevice.cpp --- a/src/kcompressiondevice.cpp +++ b/src/kcompressiondevice.cpp @@ -43,15 +43,20 @@ class KCompressionDevicePrivate { public: - KCompressionDevicePrivate() + KCompressionDevicePrivate(KCompressionDevice *q) : bNeedHeader(true) , bSkipHeaders(false) , bOpenedUnderlyingDevice(false) , bIgnoreData(false) , type(KCompressionDevice::None) + , errorCode(QFileDevice::NoError) , deviceReadPos(0) + , q(q) { } + + void propagateErrorCode(); + bool bNeedHeader; bool bSkipHeaders; bool bOpenedUnderlyingDevice; @@ -61,9 +66,23 @@ KFilterBase::Result result; KFilterBase *filter; KCompressionDevice::CompressionType type; + QFileDevice::FileError errorCode; qint64 deviceReadPos; + KCompressionDevice *q; }; +void KCompressionDevicePrivate::propagateErrorCode() +{ + QIODevice *dev = filter->device(); + if (QFileDevice *fileDev = qobject_cast(dev)) { + if (fileDev->error() != QFileDevice::NoError) { + errorCode = fileDev->error(); + q->setErrorString(dev->errorString()); + } + } + // ... we have no generic way to propagate errors from other kinds of iodevices. Sucks, heh? :( +} + KFilterBase *KCompressionDevice::filterForCompressionType(KCompressionDevice::CompressionType type) { switch (type) { @@ -88,7 +107,7 @@ } KCompressionDevice::KCompressionDevice(QIODevice *inputDevice, bool autoDeleteInputDevice, CompressionType type) - : d(new KCompressionDevicePrivate) + : d(new KCompressionDevicePrivate(this)) { assert(inputDevice); d->filter = filterForCompressionType(type); @@ -99,7 +118,7 @@ } KCompressionDevice::KCompressionDevice(const QString &fileName, CompressionType type) - : d(new KCompressionDevicePrivate) + : d(new KCompressionDevicePrivate(this)) { QFile *f = new QFile(fileName); d->filter = filterForCompressionType(type); @@ -142,6 +161,7 @@ if (!d->filter->device()->isOpen()) { if (!d->filter->device()->open(mode)) { //qCWarning(KArchiveLog) << "KCompressionDevice::open: Couldn't open underlying device"; + d->propagateErrorCode(); return false; } d->bOpenedUnderlyingDevice = true; @@ -161,20 +181,29 @@ if (!isOpen()) { return; } + d->errorCode = QFileDevice::NoError; if (d->filter->mode() == QIODevice::WriteOnly) { write(nullptr, 0); // finish writing } //qCDebug(KArchiveLog) << "Calling terminate()."; if (!d->filter->terminate()) { //qCWarning(KArchiveLog) << "KCompressionDevice::close: terminate returned an error"; + d->errorCode = QFileDevice::UnspecifiedError; } if (d->bOpenedUnderlyingDevice) { - d->filter->device()->close(); + QIODevice *dev = d->filter->device(); + dev->close(); + d->propagateErrorCode(); } setOpenMode(QIODevice::NotOpen); } +QFileDevice::FileError KCompressionDevice::error() const +{ + return d->errorCode; +} + bool KCompressionDevice::seek(qint64 pos) { if (d->deviceReadPos == pos) { @@ -374,7 +403,9 @@ int size = filter->device()->write(d->buffer.data(), towrite); if (size != towrite) { //qCWarning(KArchiveLog) << "KCompressionDevice::write. Could only write " << size << " out of " << towrite << " bytes"; - return 0; // indicate an error (happens on disk full) + d->errorCode = QFileDevice::WriteError; + setErrorString(tr("Could not write. Partition full?")); + return 0; // indicate an error } //qCDebug(KArchiveLog) << " wrote " << size << " bytes"; }