Review Request: Speed limit in ftp kio slave

[Tushar Mehta]

> On Aug. 10, 2011, 1:07 p.m., David Faure wrote:
> > kioslave/ftp/speedController.h, line 24
> > <http://git.reviewboard.kde.org/r/102267/diff/1/?file=31277#file31277line24>
> >
> >     kde_file.h isn't used in this header -> move the #include to the .cpp file.

I have used it for usleep. If I am not including it then it give me this:
"error: ‘usleep’ was not declared in this scope"


> On Aug. 10, 2011, 1:07 p.m., David Faure wrote:
> > kioslave/ftp/speedController.h, line 33
> > <http://git.reviewboard.kde.org/r/102267/diff/1/?file=31277#file31277line33>
> >
> >     trailing whitespace

acknowledged.


> On Aug. 10, 2011, 1:07 p.m., David Faure wrote:
> > kioslave/ftp/speedController.cpp, line 3
> > <http://git.reviewboard.kde.org/r/102267/diff/1/?file=31278#file31278line3>
> >
> >     Not my code :)

acknowledged.


> On Aug. 10, 2011, 1:07 p.m., David Faure wrote:
> > kioslave/ftp/speedController.cpp, line 31
> > <http://git.reviewboard.kde.org/r/102267/diff/1/?file=31278#file31278line31>
> >
> >     Make getters const, for good practice.

acknowledged.


> On Aug. 10, 2011, 1:07 p.m., David Faure wrote:
> > kioslave/ftp/speedController.cpp, line 55
> > <http://git.reviewboard.kde.org/r/102267/diff/1/?file=31278#file31278line55>
> >
> >     This doesn't seem to "add" anything, but to "set". It replaces any existing socket.
> >     
> >     Note: the naming is wrong. m_socket looks like a member variable, while "socket" is the actual member variable.
> >     
> >     I would suggest to use m_ for the actual member vars, in fact -- and for sure never for function parameters.

acknowledged.


- Tushar


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


On Aug. 9, 2011, 7:16 p.m., Tushar Mehta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102267/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2011, 7:16 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> - This patch contains the basic code which will put the limit on download speed of the ftp data transfer.
> - It is looking for "speed-limit" meta-data for deciding how much speed control is required.
> - If this meta-data is not found, code will work as it was before and no speed control related code will come into picture.
> - This patch is the most basic one which I have testing on my system and to the extent it is controlling the speed.
> - Lets say if speed limit is 30 KBps then mostly will get the avg speed around 30 to 35 KBps.
> - I am using QTime for measuring time elapsed between two socket read call and its precision is in millisecond. Looping is taking place in microsecond and thats why I am getting almost all the time 0 as time elapsed in between two calls.
> - To solve the above problem usleep is introduced to make it sync with the timer.
> 
> 
> Diffs
> -----
> 
>   kioslave/ftp/CMakeLists.txt e080b02 
>   kioslave/ftp/ftp.h 0bd375b 
>   kioslave/ftp/ftp.cpp 655524a 
>   kioslave/ftp/speedController.h PRE-CREATION 
>   kioslave/ftp/speedController.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102267/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tushar
> 
>

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