[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Bert Huijben <bert_at_qqmail.nl>
Date: Tue, 20 Nov 2012 16:18:27 +0100

> -----Original Message-----
> From: Julian Foad [mailto:julianfoad_at_btopenworld.com]
> Sent: dinsdag 20 november 2012 15:53
> To: C. MichaelPilato
> Cc: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1411671 -
> /subversion/trunk/subversion/libsvn_ra_serf/serf.c
>
> > 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.
>
> It's stupid and backwards for at least two reasons.
>
> 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?
>
> 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.

Note that this patch +- broke all the serf tests on the Windows buildbot.

The code is not needed on systems where int is 64 bit or bigger, but on
Windows int is currently 32 bit for both x86 and x64.

        Bert
Received on 2012-11-20 16:19:09 CET

This is an archived mail posted to the Subversion Dev mailing list.