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