On Wed, 24 Mar 2010, Ben Greear wrote:

> So, here it is the patch against latest git repo. I'm off for more caffeine
> right now!

Thanks! Although I have some quirks and considerations regarding it:

You introduced two warnings:

tftp.c:1213: warning: conversion to 'long int' from 'curl_off_t' may alter its
value
tftp.c:1224: warning: conversion to 'long int' from 'curl_off_t' may alter its
value

... and if you check the patch again, you reverted my recent fixes that took
away exactly those problems. sleep_time() returns a long (and it has a range
check towards the end to prevent overflows), while you introduce
Curl_sleep_time() instead that again returns a curl_off_t

Also, when you bring that function to the rest of the library, can I please
ask that you add some comments to the code that explains it? It uses a lot of
magic constants and I don't follow the logic behind all of them. Like why does
it bother with the buffer size, does that really make any noticable
difference?

Taking that sleep function to the "global" scope in transfer.c, we get your
function added to the existing speed checks. What good does it actually bring
to the code outside tftp.c? As far as I know the rate limiting already works
for non-TFTP protocol so what is this fix for? The sub-second waits until the
speed is checked again?

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html