[Kget] speed limit patches

David Faure faure at kde.org
Mon Mar 3 17:45:54 CET 2008


On Monday 03 March 2008, Manolo Valdes wrote:
> On Monday 03 March 2008 2:41:49 pm you wrote:
> >
> > Can I see the patch again? :)
> 
> sure :)
> 
> the idea is to use a new metadata "speed-limit" with a qlonglong value = speed
OK.

> some of the code that lies on HTTPslave may be implemented on the TCPSlaveBase 
> Class so others protocols that inherits TCPSlaveClass may use this feature 
> too. like sftp
> sadly ftp-slave inherits directly form SlaveBase so this feature has code on 
> both Classes (SlaveBase and TCPSlaveBase)

Hmm. Maybe kio_ftp should inherit TCPSlaveBase... I don't really know what that does though.

Comments on the patch:

+
+    /**
+     * Holds speed limit value.
+     */
+    qlonglong m_speedLimit;

You can't add a member variable in SlaveBase, that is binary incompatible. Move it to the private class
and add getter+setter.

I guess the use of usleep will break on Windows. How about using QThread::usleep() instead? (static method, so no problem)

Similarly, if you only need the elapsed time in milliseconds, why not use QTime::start() and QTime::elapsed()? I suggest
this one not for portability (I think gettimeofday is available everywhere), but for making the code much simpler.

Smaller issues:

bytesToRead() should be const.

+     *  Calculates the bytes to be readed according to the speedlimit.
The past participe of "to read" is "read", not "readed".

+     * \code
+     * // activates speed limitation through the metaData
+     * m_speedLimit = metaData("speed-limit").toULongLong();
+     * // applies the speed limitation
+     *    qlonglong size = bytesToRead();
+     *    if (size != -1)
+     *        int n = socket->read( buffer, size);
Remove the "int", doesn't make sense unless you add { ... }

+     * \endcode 
+     * @return the number of bytes to be readed.
read.



-- 
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).


More information about the Kget mailing list