Review Request: [PATCH] Make the FTP ioslave emit mime-type of the content before actually reading it

[Dawit Alemayehu]

> 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.

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.


> On April 18, 2011, 8:31 p.m., David Faure wrote:
> > kioslave/ftp/ftp.cpp, line 2425
> > <http://git.reviewboard.kde.org/r/101149/diff/3/?file=14732#file14732line2425>
> >
> >     Are you sure that peek() is cumulative? I.e. that peek(100) + peek(100) doesn't give you the same data twice? I thought it would, since it "rewinds" after giving you the data.

Oops, indeed!  After all that is exactly the difference between "read" and "peek" in the first place. Ah well perhaps I should go back to bed now. Will correct this debacle and post a corrected patch.


- Dawit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101149/#review2733
-----------------------------------------------------------


On April 18, 2011, 8:12 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, 8:12 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110418/5efc051c/attachment.htm>