D20519: Fix double delete on broken files

David Faure noreply at phabricator.kde.org
Sun Apr 14 10:48:08 BST 2019


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 any regressions.
  I managed to write a unittest for this one, please integrate it into your commit:
  
  http://www.davidfaure.fr/2019/ktar_unittest.diff
  http://www.davidfaure.fr/2019/tar_emptyfile_missingdir.tar.gz
  
  The tar file was created like this (this info should go into the commit log)
  
  mkdir dir
  touch dir/foo
  tar cf tar_emptyfile_missingdir.tar dir/foo
  rm -f dir/foo
  mkdir dir/foo
  touch dir/foo/file
  tar rf tar_emptyfile_missingdir.tar dir/foo/file
  gzip tar_emptyfile_missingdir.tar

INLINE COMMENTS

> karchive.cpp:77
> +
> +    // Returns in containingDirectory the directory that actually contains the returned entry
> +    const KArchiveEntry *entry(const QString &_name, KArchiveDirectory **containingDirectory) const

A cleaner solution is to return a small struct with the two things we want to return.
This would also remove the need for a "dummy" variable.

REPOSITORY
  R243 KArchive

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

To: aacid, dfaure
Cc: dfaure, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190414/1d1c0007/attachment.html>


More information about the Kde-frameworks-devel mailing list