[Differential] [Accepted] D4422: Fix KCompressionDevice to work with Qt >= 5.7

David Faure noreply at phabricator.kde.org
Sat Feb 4 12:42:28 UTC 2017


dfaure accepted this revision.
dfaure added a reviewer: dfaure.
dfaure added a comment.
This revision is now accepted and ready to land.


  Thanks for the fix.
  
  I had difficulties to understand "Don't use QIODevice:pos to track our pos, doesn't work.", so I investigated a bit and indeed QIODevice has d->pos (the publically visible pos) and d->devicePos (where we are in the actual device, after the last read), so for instance after seeking back we have d->pos==0 and d->devicePos==5.
  
  The problem is, subclasses can't really access d->devicePos, they can only set it by calling QIODevice:seek().
  
  In our own seek() we need to know by how many bytes forward we are moving in terms of devicePos, and we can't know that unless we keep our own member variable indeed. We'd want ioIndex=d->devicePos (at the top of the method, before calling QIODevice::seek), NOT ioIndex=pos(). But we can't access that.
  
  So your patch adds a member variable which duplicates QIODevice::d->devicePos, but there's no other solution without adding more API to QIODevice.
  
  I deem QIODevice API incomplete, and your patch correct given the circumstances.
  
  PS: next time please include the autotest in the commit rather than committing it before, although that's a nice trick to make me jump at the CI failure and look around for the corresponding fix :)

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D4422

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: aacid, dfaure
Cc: dfaure, anthonyfieroni, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170204/c2df8a3a/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list