D23708: [CopyJob] Fix crash when copying all files is skipped for an already existing dir

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Thanks for the fix!
  
  Do you feel like writing a unittest for it?
  JobTest::chmodFileError shows how to use PredefinedAnswerJobUiDelegate to simulate the user pressing "Skip" in the dialog.
  Write the test without your fix applied, so that you are sure it triggers the crash, before re-applying the fix.

INLINE COMMENTS

> copyjob.cpp:1640
>  
> -        if ((*it).size > ((1ul << 32) - 1)) { // ((1ul << 32) - 1) = 4 GB
> +        // if _all_ elements in "files" have been skipped, the last files.erase() will return
> +        // files.end(), bug 408350

This comment only needs to be in the unittest, not in the code.

If every bugfix leads to two lines of comments, the code becomes unreadable.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190905/9b6e39fc/attachment-0001.html>