On Tuesday 24 November 2015 23:38:28 David Faure wrote: > On Monday 23 November 2015 12:11:12 Luiz Romário Santana Rios wrote: > > 2015-11-21 21:02 GMT-03:00 David Faure <fa...@kde.org>: > > > > > > This is an automatically generated e-mail. To reply, visit: > > > https://git.reviewboard.kde.org/r/125974/ > > > > > > On November 7th, 2015, 9:25 a.m. UTC, David Faure wrote: > > > > > > BTW why create a KTar on top of a KCompressionDevice? KTar is able to > > > handle compression automatically all by itself... and that's exactly the > > > case where it will use a tempfile for the uncompressed data, making > > > seeking work. This patch is unnecessary if you use KTar the intended way: > > > if the filename doesn't end with .gz or .bz2, specify the mimetype > > > explicitly in the KTar constructor. > > > > > > On November 21st, 2015, 7:11 p.m. UTC, Romário Rios wrote: > > > > > > KCompressionDevice is supposed to be used when you want to extract data > > > from a QIODevice directly instead of a file -- KTar only handles > > > compression automatically if you pass it a filename. ATM, > > > KCompressionDevice doesn't seem to work properly with many QIODevices -- > > > not even with QBuffer. > > > > > > KCompressionDevice should certainly work on top of a QBuffer. I just > > > committed a unittest that shows it working (but of course there might be > > > a bug in some other way to use it, feel free to show me in which case it > > > doesn't work). > > > > Please take a look at my patch from the #125941 review request: > > https://git.reviewboard.kde.org/r/125941/diff/2#2 > > It seems none of the test*BufferedNetworkReplyDevice tests work. > > Ah, I see. I debugged the issue and came up with this patch: > > diff --git a/src/kcompressiondevice.cpp b/src/kcompressiondevice.cpp > index 77d7deb..aeefe6a 100644 > --- a/src/kcompressiondevice.cpp > +++ b/src/kcompressiondevice.cpp > @@ -196,7 +196,7 @@ bool KCompressionDevice::seek(qint64 pos) > return d->filter->device()->reset(); > } > > - if (ioIndex > pos) { // we can start from here > + if (ioIndex < pos) { // we can start from here > pos = pos - ioIndex; > } else { > // we have to start from 0 ! Ugly and slow, but better than the > previous > > > This fixes the runtime warnings but somehow openArchive returns false, I'm > confused as to why, > but it's time for bed, more next time.
Found another bug in this same method, clearly KCompressionDevice::seek had never been called before your unittest ;) Here's the fix: http://commits.kde.org/karchive/b31b66f4ac0f0941fbf784cc6aeba7190dbd78bf Now the *Buffered methods of your unittests pass. Can you commit that unittest, at least the *Buffered methods? Don't forget to add a license+copyright header. Ah and rename these methods to remove "NetworkReply" from their name, they really test KCompressionDevice with QBuffer, there's no networkreply in the mix. Now that things work with a QBuffer, we can talk about QNetworkReply again :) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 _______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
