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

On Wed, Aug 06, 2014 at 09:05:41PM +0200, Stefan Fuhrmann wrote:
> It seems to me that the correct approach would be
> adding overflow checks to svn__strto*.

How do you intend to do this correctly?

The way I see it, we need to change the svn__strto* API for this
because currently there is no way to return an error from it.
You modelled the interface on strtol() but you didn't put in an
equivalent for how strtol() uses errno to communicate overflow
conditions to the caller. From the caller's point of view the
svn__strto* interfaces only cover the conditions where no error occurs!

If we're going to make API changes anyway we might just as well remove
the new interface and use the existing svn_cstring_atoi* APIs for this
purpose. These interfaces already check for overflow and return errors,
forcing callers to handle them or raise a red flag by ignoring them.

How about I commit my patch, and then someone else (you? Bert?) tunes the
implementations of svn_cstring_atoi*, perhaps reusing code from svn__strto*
but keeping the API promises intact?
I think this is what should have happened in the first place.

And I'll work on providing an svn_cstring_atoi* variant that returns *endp,
and will try to replace remaining strtol() calls with it. If they're slow
for parsing revision numbers they're bound to be slow for other things too.

Does that work for you?
Received on 2014-08-06 23:24:15 CEST