<table><tr><td style="">dfaure accepted this revision.<br />dfaure added a reviewer: dfaure.<br />dfaure added a comment.<br />This revision is now accepted and ready to land.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D4422" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>Thanks for the fix.</p>
<p>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.</p>
<p>The problem is, subclasses can't really access d->devicePos, they can only set it by calling QIODevice:seek().</p>
<p>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.</p>
<p>So your patch adds a member variable which duplicates QIODevice::d->devicePos, but there's no other solution without adding more API to QIODevice.</p>
<p>I deem QIODevice API incomplete, and your patch correct given the circumstances.</p>
<p>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 :)</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R243 KArchive</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D4422" rel="noreferrer">https://phabricator.kde.org/D4422</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>aacid, dfaure<br /><strong>Cc: </strong>dfaure, anthonyfieroni, Frameworks<br /></div>