2015-11-25 5:08 GMT-03:00 David Faure <fa...@kde.org>: > 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 ;)
Thanks for the effort put into looking for that bug. > > 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. I uploaded a new diff to reviewboard. Btw, the commit 0f0230f7d2feeca7ed00072e7b17b24c14f53698 ("Fix clang warnings") makes the compilation fail in my machine. In the KGzipFilter::setInBuffer() method, you change a C-cast to (Bytef *) to a reinterpret_cast<const Bytef *>, but you're attributing it to d->zStream.next_in, which is a char *. To make it work, I had to change it to const_cast<Bytef *>(reinterpret_cast<const Bytef *>(data)). Is there a better way to do this. > > Now that things work with a QBuffer, we can talk about QNetworkReply again :) Good :). I already started talking about it in my original thread to the mailing list, check it out. > > -- > David Faure, fa...@kde.org, http://www.davidfaure.fr > Working on KDE Frameworks 5 > -- Luiz Romário Santana Rios _______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
