Review Request 125974: Make KTar KCompressionDevice-friendly

[David Faure]

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 <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.

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, faure at kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5