D20519: Fix double delete on broken files

2019-04-14 Thread Albert Astals Cid
aacid closed this revision. REPOSITORY R243 KArchive REVISION DETAIL https://phabricator.kde.org/D20519 To: aacid, dfaure Cc: dfaure, kde-frameworks-devel, michaelh, ngraham, bruns

D20519: Fix double delete on broken files

2019-04-14 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. The double star looks very C-ish to me, but OK, I just realized that the recursive call itself would probably look more cumbersome, so I won't insist. INLINE COMMENTS > karchive.cpp:871 >

D20519: Fix double delete on broken files

2019-04-14 Thread Albert Astals Cid
aacid added a comment. In D20519#450066 , @dfaure wrote: > Any input on my "cleaner solution: return a two-field structs" suggestion? I can do it if you want, but i feel that for a function that is private and called in one just place the

D20519: Fix double delete on broken files

2019-04-14 Thread David Faure
dfaure added a comment. Oops indeed, I stopped working on the unittest when I managed to make it reproduce the crash. Thanks for integrating it and fixing it. Any input on my "cleaner solution: return a two-field structs" suggestion? REPOSITORY R243 KArchive REVISION DETAIL https://

D20519: Fix double delete on broken files

2019-04-14 Thread Albert Astals Cid
aacid updated this revision to Diff 56218. aacid added a comment. update with test from dfaure + local tweak REPOSITORY R243 KArchive CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20519?vs=56161&id=56218 BRANCH arcpatch-D20519 REVISION DETAIL https://phabricator.kde.org/D20

D20519: Fix double delete on broken files

2019-04-14 Thread Albert Astals Cid
aacid added a comment. In D20519#449595 , @dfaure wrote: > Good catch. > I'd still prefer if such changes would come with a unittest, because it will be hard otherwise to immediately detect (while developing) that changes don't introduce any

D20519: Fix double delete on broken files

2019-04-14 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Good catch. I'd still prefer if such changes would come with a unittest, because it will be hard otherwise to immediately detect (while developing) that changes don't introduce

D20519: Fix double delete on broken files

2019-04-13 Thread Albert Astals Cid
aacid added a subscriber: dfaure. REPOSITORY R243 KArchive REVISION DETAIL https://phabricator.kde.org/D20519 To: aacid Cc: dfaure, kde-frameworks-devel, michaelh, ngraham, bruns

D20519: Fix double delete on broken files

2019-04-13 Thread Albert Astals Cid
aacid created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. aacid requested review of this revision. REVISION SUMMARY findOrCreate has some code that tries to recover broken files. That code will delete an empty file if later it looks li