Review Request 125974: Make KTar KCompressionDevice-friendly

[David Faure]

On Monday 23 November 2015 12:11:12 Luiz Romário Santana Rios wrote:
> 2015-11-21 21:02 GMT-03:00 David Faure <faure at 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.

> > Also, it's not true that KTar only handles compression if given a filename. You can also pass it a mimetype like application/x-gzip and then it will use compression.
> 
> What I meant was that, besides the QIODevice * constructor, KTar only
> has a constructor taking a filename and (optionally) a mimetype.

Oh I see what you mean. Forgot my own docu from years ago.

OK. We could add a new ctor for convenience, but yeah overall it's about making this work,
KTar on top of KCompressionDevice (whether it's created externally or internally).

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5