Re: svn commit: r1411671 - /subversion/trunk/subversion/libsvn_ra_serf/serf.c

On 11/20/2012 09:53 AM, Julian Foad wrote:
>> Author: cmpilato
>
>>
>> URL: http://svn.apache.org/viewvc?rev=1411671&view=rev
>> Log:
>> Minor logic simplification.
>>
>> * subversion/libsvn_ra_serf/serf.c
>> (load_config): Simplify timeout calculations, since we know
>> DEFAULT_HTTP_TIMEOUT is non-negative and we know the
>> get-time-from-config path errors out on negative values.
>
> Oops -- that logic was there to catch the fact that the default 3600 seconds goes negative in the 32-bit variable it's assigned to.

Ah! Shoot. Sorry about the broken tests.

> 1) Bert says on IRC, "The timeout value in neon was used for a completely different purpose: In neon: complete request timeout, in serf: chunk received timeout. Default http timeout is 30 or 60 minutes. A bit long for waiting for a tcp segment."
>
> So I guess we should set the default to something more sane such as ... one minute?

5 minutes -- the same as Apache httpd's default timeout for receiving data
from a client?

> 2) Why store microseconds (I know it's APR's standard for time values) and allocate a 32-bit variable for it? Either use a bigger variable or just store the number of whole seconds.

Agreed. This is just silly.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development