> On Oct. 19, 2015, 7:28 a.m., David Faure wrote: > > kio/kio/job.cpp, line 2671 > > <https://git.reviewboard.kde.org/r/125613/diff/3/?file=411228#file411228line2671> > > > > Same question here. As the comment says, "the result is still OK", > > which is why I didn't set an error code here. Please explain why this is > > needed.
The comment exactly says: (If we can't list a subdir, the result is still ok). I interpret it like the result is not ok if the error is not about being unable to list a directory. > On Oct. 19, 2015, 7:28 a.m., David Faure wrote: > > kio/kio/copyjob.cpp, line 579 > > <https://git.reviewboard.kde.org/r/125613/diff/3/?file=411227#file411227line579> > > > > We don't want to abort the copy just because one unreadable folder was > > found. We want to tell the user (hence the warning line below) and keep > > going. > > This emulates what `cp -a ~/src ~/dst` does. > > > > OK, you're not exactly aborting, but you're still setting an error - > > even though everything else got copied. I don't see why this is necessary. > > Is it just to get an error box, or does it fix behaviour? I made it to make the test pass. But a MessageBox shows up later telling that the folder could not be copied. It is not actually needed. I have changed the test so that it checks for any entries emmited by warning. - Alberto ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87048 ----------------------------------------------------------- On Oct. 17, 2015, 2:02 p.m., Alberto Jiménez Ruiz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125613/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2015, 2:02 p.m.) > > > Review request for kdelibs, Albert Astals Cid and David Faure. > > > Bugs: 333436 > http://bugs.kde.org/show_bug.cgi?id=333436 > > > Repository: kdelibs > > > Description > ------- > > Race condition and error notification loss in ListJob > > > Diffs > ----- > > kio/kio/copyjob.cpp e6c3817 > kio/kio/job.cpp 91712e3 > kio/kio/jobclasses.h d771cfe > kio/tests/jobtest.h d3c552e > kio/tests/jobtest.cpp ee2677a > > Diff: https://git.reviewboard.kde.org/r/125613/diff/ > > > Testing > ------- > > Update 1, added unittest > Changed condition of listjob slotresult finishing (When executed as a > synchronous job, gets stuck in a eventloop) > Added setError to copyjob. > > > Tests done on Kubuntu 15.10: > > With this folder structure in ~: > src > ######c (Folder) > ##############b (Folder chmod'ed to 700 and owned by root) > #######################d (10MB file made of /dev/urandom data) > ##############a (10MB file made of /dev/urandom data) > > Then, (as a normal user execute: "kioclient ~/src ~/dst") > > Expected result: Notification that some files were not able to be copied > (Cannot access "b" folder) > Result with latest kdelibs: Silent copying where b folder is owned by user > but nothing inside. > ---When all kdebugdialog flags are set, a backtrace is also seen. > > subError notifications for listjob subjobs are lost, Copyjob uses this signal > to skip data. > > Also, this fix prevents a race condition. > > ListJob has another ListJob as a subjob. > > Chaild fails for whatever reason and calls error, which calls slotError, > which calls slotsFinished and emitResult. > The result of the child gets collected by parent. slotResult of parent gets > called, removes subjob and emitsresult because there are no subjobs left. > ---That's fine as long as the slave associated with the parent emits finished > before the child fails. If it doesn't, parent calls emitsResult twice. > > > Result after patch: kioclient shows a MessageBox telling the copy operation > could not be completed. > > > Thanks, > > Alberto Jiménez Ruiz > >
