> On Oct. 16, 2012, 11:50 a.m., David Faure wrote: > > kioslave/ftp/ftp.cpp, line 1491 > > <http://git.reviewboard.kde.org/r/106636/diff/3/?file=88065#file88065line1491> > > > > This is slow iteration, since it has to modify the container every > > time. There's no reentrancy here, why use this complicated approach, when a > > simple Q_FOREACH could do?
I did not want to use Q_FOREACH here because the items we are iterating over need to be modified by calls to fixupEntryName. I wanted to avoid copying each item on the list if I could, but your point is valid. Modifying the list would be more expensive ; so I have opted for using a standard for-loop to iterate over the loop and access the list items using the [] operator. That way we perform neither on of the more expensive operations. > On Oct. 16, 2012, 11:50 a.m., David Faure wrote: > > kioslave/ftp/ftp.cpp, line 1493 > > <http://git.reviewboard.kde.org/r/106636/diff/3/?file=88065#file88065line1493> > > > > Once bFound is true, we're done. Why not break out of the loop, then? > > (and remove this if()) ? > > > > This is unlike the loop you're copying from, which has to keep > > processing the directory listing from the server. - Dawit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106636/#review20451 ----------------------------------------------------------- On Oct. 10, 2012, 3:55 p.m., Dawit Alemayehu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106636/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2012, 3:55 p.m.) > > > Review request for kdelibs and David Faure. > > > Description > ------- > > The attached patch fixes a regression caused by a commit to fix bug# 88575. > Namely, it fixed a problem where filenames with whitespaces in them were not > handled correctly by kio_ftp. That is because the filenames were > automatically trimmed when read from the directory. However, the fix then > re-introduced the original bug and the reason why names were automatically > trimmed in the first place. Some ftp servers add bogus whitespace between the > date and filename in their listings. Hence, we need need to fix both of these > opposing issues without breaking the other. This patch tries to do just that > by actually validating each name entry that starts with a whitespace. That > way the correct name is sent to the client. > > > This addresses bug 300988. > http://bugs.kde.org/show_bug.cgi?id=300988 > > > Diffs > ----- > > kioslave/ftp/ftp.h 2465a4b > kioslave/ftp/ftp.cpp 26be283 > > Diff: http://git.reviewboard.kde.org/r/106636/diff/ > > > Testing > ------- > > - Create 150 folders and 150 text files whose name starts with a whitespace > in a directory. > - Start your preferred ftp server (I tested with vsftpd) on your local > machine and login. > - Make sure you can browse the directory that contains the text files and > folders you created above just as quickly as you can other ftp sites. > - Rename some of the folders and text files by remove the leading whitespace > from them and check if that is handled properly. > > > Thanks, > > Dawit Alemayehu > >
