D14913: KCompressionDevice: propagate errors from QIODevice::close()BUG: 397545
dfaure created this revision. dfaure added a reviewer: cullmann. Herald added a project: Frameworks. Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks. dfaure requested review of this revision. REVISION SUMMARY QFile::close() doesn't return a value, but sets an error string instead, we need to check for that and set it in KCompressionDevice so apps can check for that in turn. TEST PLAN Saving into /tmp/mnt/test.gz with kate, with the setup described in https://phabricator.kde.org/D14890 REPOSITORY R243 KArchive BRANCH master REVISION DETAIL https://phabricator.kde.org/D14913 AFFECTED FILES src/kcompressiondevice.cpp To: dfaure, cullmann Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D14913: KCompressionDevice: propagate errors from QIODevice::close()BUG: 397545
cullmann accepted this revision. cullmann added a comment. This revision is now accepted and ready to land. Propagating the error is good in any case. Will later adapt my patch to use it, like you proposed. Still, my crash does happen independent of this I assume. REPOSITORY R243 KArchive BRANCH master REVISION DETAIL https://phabricator.kde.org/D14913 To: dfaure, cullmann Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D14913: KCompressionDevice: propagate errors from QIODevice::close()BUG: 397545
cullmann added a comment. Hmm, actually, that has other issues. You can't check errorString().isEmpty() for checking if an error is there. I just commited my other patch with the addition of that code (and was to dumb to run make test) e.g. even after constructing a QFile, you already have: const KCompressionDevice::CompressionType type = KFilterDev::compressionTypeForMimeType(m_mimeTypeForFilterDev); QScopedPointer saveFile((type == KCompressionDevice::None) ? static_cast(new QFile(filename)) : static_cast(new KCompressionDevice(filename, type))); qDebug() << "lalal file " << filename << "failed with error" << saveFile->errorString(); > = lalal file "/home/cullmann/kde/build/frameworks/ktexteditor/jsdklfjsdklffsf" failed with error "Unknown error" REPOSITORY R243 KArchive BRANCH master REVISION DETAIL https://phabricator.kde.org/D14913 To: dfaure, cullmann Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D14913: KCompressionDevice: propagate errors from QIODevice::close()BUG: 397545
cullmann requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R243 KArchive REVISION DETAIL https://phabricator.kde.org/D14913 To: dfaure, cullmann Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D14913: KCompressionDevice: propagate errors from QIODevice::close()BUG: 397545
dfaure updated this revision to Diff 39963. dfaure added a comment. Rework as discussed Updating D14913: KCompressionDevice: propagate errors from QIODevice::close() = BUG: 397545 REPOSITORY R243 KArchive CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14913?vs=39950&id=39963 BRANCH master REVISION DETAIL https://phabricator.kde.org/D14913 AFFECTED FILES autotests/kcompressiondevicetest.cpp autotests/kcompressiondevicetest.h src/kcompressiondevice.cpp src/kcompressiondevice.h To: dfaure, cullmann Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D14913: KCompressionDevice: propagate errors from QIODevice::close()BUG: 397545
cullmann accepted this revision. cullmann added a comment. This revision is now accepted and ready to land. Looks reasonable, given we must live with the constraint of having error() atm only in QFileDevice. REPOSITORY R243 KArchive BRANCH master REVISION DETAIL https://phabricator.kde.org/D14913 To: dfaure, cullmann Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D14913: KCompressionDevice: propagate errors from QIODevice::close()BUG: 397545
dfaure closed this revision. REPOSITORY R243 KArchive REVISION DETAIL https://phabricator.kde.org/D14913 To: dfaure, cullmann Cc: kde-frameworks-devel, michaelh, ngraham, bruns
