subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fuhrmann <stefan.fuhrm...@wandisco.com>
Subject Re: [PATCH] remove svn__strtol() and svn__strtoul()
Date Wed, 06 Aug 2014 19:05:41 GMT
On Wed, Aug 6, 2014 at 8:05 PM, Stefan Sperling <stsp@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.

Mime
View raw message