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

Reply via email to