D28669: make CopyJob non-recursive

Méven Car noreply at phabricator.kde.org
Thu Apr 9 10:43:55 BST 2020


meven added a comment.


  Currently the code does not handle the other call paths to `statNextSrc()` that would need to be adapted as well. We will need to call `statCurrentSrc()` after `statNextSrc()` basically there.
  In slotResultRenaming and slotResult.
  Currently, those code path would cause the finish the copy prematurely.
  
  The tests currently don't pass, they just hang.

INLINE COMMENTS

> dfaure wrote in copyjob.cpp:809
> Why did you remove this call?
> 
> Your patch has no context (please use `arc` to upload patches to phabricator), but I can see here that this is in the CopyJobPrivate::skipSrc method. Did you test skipping files? This isn't mentioned in your test plan.
> 
> Does the unittest jobtest still pass?

This is in `statNextSrc()`
This is the main point of recursion, so it this where this needs to change to break the recursion in coherence with the `continue`s added in `CopyJobPrivate::statCurrentSrc`

> copyjob.cpp:849
>              files.append(info);   // Files and any symlinks
>              statNextSrc(); // we could use a loop instead of a recursive call :)
> +            continue;

Remove comment

REPOSITORY
  R241 KIO

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

To: McPain, #frameworks, dfaure, meven, ahmadsamir
Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200409/cfe2c9ca/attachment.html>


More information about the Kde-frameworks-devel mailing list