Differential D19420

Fix KCompressionDevice::atEnd() for uncompressed .gz files
Needs ReviewPublic
Actions

Authored by croick on Feb 28 2019, 1:32 PM.

Details

Reviewers

broulik

Group Reviewers

Frameworks

Summary

return KFilterBase::End if KGzipFilter::uncompress_noop() finished reading
reset result to KFilterBase::Ok if there is data remaining
fixes 96083005e90e

Test Plan

load gzipped files (size < and > than BUFFER_SIZE = 8192 bytes) using kate and labplot
load unzipped files with .gz suffix (size < and > than 8192 bytes) using kate and labplot
load bzipped files with kate

Diff Detail

Lint

Lint Skipped

Unit

Unit Tests Skipped

croick created this revision.Feb 28 2019, 1:32 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 28 2019, 1:32 PM

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

croick requested review of this revision.Feb 28 2019, 1:32 PM

Works fine here, thanks, but I don't feel qualified enough to comment on this patch, I added Frameworks reviewers.

With a unit test it would be much easier to accept the patch ;)

add unit test, reset bNeedHeader

In D19420#422043, @apol wrote:

With a unit test it would be much easier to accept the patch ;)

... and to identify a remaining bug.
The header is now read again after re-initialization.

Would you accept now @apol?

In D19420#441163, @croick wrote:

Would you accept now @apol?

Or now?

Looks good to me, have you checked that it's now not breaking what @broulik found?

In D19420#460766, @apol wrote:

Looks good to me, have you checked that it's now not breaking what @broulik found?

Yes, I tested loading systemsettings.svg (/usr/share/icons/breeze/apps/48/systemsettings.svg) by using Kate and renaming the suffix to .svgz. With my previous commit 96083005e90e, the file would not be loaded entirely. Using this patch, all of the file is loaded.
I don't compile plasma myself, so I didn't check whether the icons are displayed now. But the reason for not rendering the icons is gone.

Revision Contents
Changeset List

		Path
A	M	autotests/data/noop_big.gz (2000 lines)
A	M	autotests/data/noop_small.gz (1 line)
M		autotests/kfiltertest.h (2 lines)
M		autotests/kfiltertest.cpp (28 lines)
M		src/kcompressiondevice.cpp (2 lines)
M		src/kgzipfilter.cpp (2 lines)