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

On 07.08.2014 14:35, Stefan Sperling wrote:
> On Thu, Aug 07, 2014 at 02:27:15PM +0200, Branko Čibej wrote:
>> On 07.08.2014 08:52, Stefan Sperling wrote:
>>> On Thu, Aug 07, 2014 at 01:34:40AM +0200, Branko Čibej wrote:
>>>> On 06.08.2014 23:23, Stefan Sperling wrote:
>>>>> 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?
>>>> You'd be silently changing the semantics of public APIs. These are not
>>>> currently documented to be equivalent to strtol & co., but they're
>>>> implemented in terms of apr_strtol & co., which are just replacements
>>>> for those platforms that do not have the standard <string.h> header.
>>> So?
>> So we can't go silently changing the semantics of our public APIs.
>>
>> Unless you can convince someone (me?) that this is a topic for API errata.
>>
>> -- Brane
> I'd argue that it's an implementation detail.

Changing visible semantics of a public API cannot in any imaginable way
be interpreted as an implementation detail.

> In fact, I wasn't aware of this when I wrote the code! So it's clearly
> unintended. These functions are supposed to expect strings that contain
> numbers, not whitespace.

I don't care. So our documentation does not properly describe the
semantics of thefunction, and so what? Anyone who uses it will sooner or
later notice that it (surprise, surprise!) behaves exactly like the
standard strtol function (plus some improvements), and It's completely
reasonable to assume that this was the intent of the API.

We're allowed to revise these APIs. If that would really hurt, and you
can make a case that our API consumers won't be hurt by it, we can even
issue API errata and change the implementation to not use isspace() but
use svn_ctype_isspace() instead (which is not locale-dependent).

But we absolutely are not allowed to change the semantics without at
least issuing an errata notice, and only in a minor or major release;
i.e., we can do this for 1.9 but cannot backport to 1.8. For 1.8, we
/can/// make the private "optimized" functions safer.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane_at_wandisco.com