----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106636/#review20451 -----------------------------------------------------------
kioslave/ftp/ftp.cpp <http://git.reviewboard.kde.org/r/106636/#comment16140> 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? kioslave/ftp/ftp.cpp <http://git.reviewboard.kde.org/r/106636/#comment16141> 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. kioslave/ftp/ftp.cpp <http://git.reviewboard.kde.org/r/106636/#comment16144> factorizing this code rather than duplicating it would have been nice kioslave/ftp/ftp.cpp <http://git.reviewboard.kde.org/r/106636/#comment16143> If it can be empty, then this should be checked before calling at(0) a few lines above... kioslave/ftp/ftp.cpp <http://git.reviewboard.kde.org/r/106636/#comment16142> same - David Faure 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 > >
