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

Re: [PATCH] remove svn__strtol() and svn__strtoul()

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Wed, 6 Aug 2014 21:05:41 +0200

On Wed, Aug 6, 2014 at 8:05 PM, Stefan Sperling <stsp_at_elego.de> wrote:

> On Wed, Aug 06, 2014 at 07:18:49PM +0200, Branko Čibej wrote:
> > On 06.08.2014 18:47, Bert Huijben wrote:
> > > Some time ago I switched a few of the calls mentioned here around to
> > > get a huge performance improvement on Windows. The standard integer
> > > parse functions start by skipping whitespace, and only then parse the
> > > integer. But whatever is whitespace is locale dependent, and checking
> > > for that from the CTY is really slow on Windows.
> > > (If I remember correctly these are exactly the parse id functions you
> > > are about to change here)
> >
> > In that case, these functions must still perform sanity checks.
>
> If we can't use strtol for this purpose on Windows, then I think we should
> introduce our own replacement only on Windows. And also make sure our
> replacement is not any less correct in terms of how strtol() behaves
> otherwise.
>

I'm +1 for better error checking, but I think that using
a different code path just for Windows is a bad idea.
Either the code is good enough, then it should be used
on all platforms to increase / not hurt our test coverage.
Or it is not up to snuff, then we should not use it anywhere.

It seems to me that the correct approach would be
adding overflow checks to svn__strto*.

-- Stefan^2.
Received on 2014-08-06 21:06:10 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.