> On April 18, 2011, 8:31 p.m., David Faure wrote: > > kioslave/ftp/ftp.cpp, line 2405 > > <http://git.reviewboard.kde.org/r/101149/diff/3/?file=14732#file14732line2405> > > > > If m_size is UnknownSize and the file is less than 1024 bytes, this > > code is going to hit the timeout, waiting for data to come in, that will > > never come in. > > > > I think that once waitForReadyRead was called once, the loop should > > break (after peeking, of course), if m_size==Unknown. > > Dawit Alemayehu wrote: > That won't happen because we also check the returned size of peek: > > if (bytesRead == 0 || bytesLeft == 0) { > break; > } > > so if the file size is < 1024, but m_size is unknown then the entire file > the loop will break once it reads the entire file from buffer. I already > tested that this works with our trusty README file of 404 bytes from the KDE > ftp site. > > David Faure wrote: > Sorry, I don't get it. m_size=1024 (because unknown), real file size=404, > bytesRead=404, bytesLeft=620 -> no break -> we go to "waitForReadyRead" until > timeout. > > Dawit Alemayehu wrote: > See new patch. I now handle that case too ; so you won't get the extra > timeout condition. If bytesRead == totalSize (which is really the size we > want), the it will break. Sorry about misreading your response. I thought you > said it will be in an infinite loop. :) > > David Faure wrote: > Nope, if m_size is UnknownSize, then we don't know the size we want. 1024 > might be impossible to ever reach. > The new code makes sense, except in the case where m_size == UnknownSize. > In that case, you need to break at the bottom of the while loop, I don't see > another solution.
ahhh... you are indeed correct. Fixed. I added a check for m_size == UnknownSize to the if statement that results in a break. - Dawit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101149/#review2733 ----------------------------------------------------------- On April 18, 2011, 9:28 p.m., Dawit Alemayehu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101149/ > ----------------------------------------------------------- > > (Updated April 18, 2011, 9:28 p.m.) > > > Review request for kdelibs and David Faure. > > > Summary > ------- > > The attached patch changes the ftpGet function such that it emits the > mime-type of the content it is about to read, before starting to read it. > That way kio_ftp will work correctly if and when it is put on hold to be > reused from another application. Note that the patch uses QIODevice::peek to > achieve this goal. > > > Diffs > ----- > > kioslave/ftp/ftp.h 9814c10 > kioslave/ftp/ftp.cpp 64f43d8 > > Diff: http://git.reviewboard.kde.org/r/101149/diff > > > Testing > ------- > > > Thanks, > > Dawit > >
