sandsmark created this revision.
sandsmark added reviewers: dfaure, apol.
sandsmark added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
sandsmark requested review of this revision.
REPOSITORY
R243 KArchive
REVISION DETAIL
https://phabricator.kde.org/D10551
AFF
sandsmark added a dependent revision: D10543: fix crashing with duplicate
entries.
REPOSITORY
R243 KArchive
REVISION DETAIL
https://phabricator.kde.org/D10551
To: sandsmark, dfaure, apol
Cc: #frameworks, michaelh
dfaure added inline comments.
INLINE COMMENTS
> karchivetest.cpp:1169
> +
> +QVERIFY(zip.open(QIODevice::ReadOnly));
> +}
Maybe do a directory listing too, to check if the duplicate name appears twice?
REPOSITORY
R243 KArchive
REVISION DETAIL
https://phabricator.kde.org/D10551
To: san
sandsmark updated this revision to Diff 27666.
sandsmark added a comment.
fix comment from dfaure
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10551?vs=27282&id=27666
REVISION DETAIL
https://phabricator.kde.org/D10551
AFFECTED FILES
autotests/data/out.epub
autotests/karchiv
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
Thanks - almost there.
INLINE COMMENTS
> karchivetest.cpp:1172
> +int metaInfCount = 0;
> +for (const QString &entryName : zip.directory()->entries()) {
> +if (ent
sandsmark updated this revision to Diff 28314.
sandsmark added a comment.
use a local const copy of entries
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D10551?vs=27666&id=28314
REVISION DETAIL
https://phabricator.kde.org/D10551
AFFECTED FILES
data/out.epub
karchivetest.cpp
sandsmark marked an inline comment as done.
sandsmark added a comment.
test file was from a customer, but it's from a public
INLINE COMMENTS
> dfaure wrote in karchivetest.cpp:1172
> This detaches the list, put it in a const local variable.
>
> (standard range-for trap with Qt containers)
t
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
Yeah but good practice is better applied everywhere, to avoid showing bad
examples to others ;)
Thanks.
REVISION DETAIL
https://phabricator.kde.org/D10551
To: sandsmark, dfaure, apo
This revision was automatically updated to reflect the committed changes.
sandsmark marked an inline comment as done.
Closed by commit R243:6959a256f777: autotests: add crashing test with duplicate
names (authored by sandsmark).
CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D10551?vs=2831
