On 14.02.2012 13:26, Julian Foad wrote:
> For review, please.
>
> We discovered some bugs recently [1,2] with use of svn_string.h functions, where space for the terminating null character was sometimes not being allocated. The attached patch file contains several changes in this area, which are all somewhat together although I'll commit them in two or more separate parts. In summary:
>
> * Introduce revved API svn_stringbuf_ensure2() that promises to make space for NUL.
>
> * Make the old svn_stringbuf_ensure() provide space for NUL even though it doesn't promise to do so, to help remaining buggy callers to 'just work', since doing so is harmless and it was our inconsistent API that led to the misunderstanding.
Please explain again, why do we need the revved _ensure2 at all? Can you
think of any possible way that even marginally sane code would break if
we just fixed the docs and behaviour of _ensure?
You have to rev when you introduce ABI changes or incompatible semantic
changes, but ... this change is not incompatible, as you point out
yourself. The worst that can happen is that well-behaved library users
will allocate a few bytes more memory when using a newer library. So what?
Just treat this as an API spec bug, fix the existing function and be
done with it. 5 minutes' work instead of never-ending code churn.
-- Brane
Received on 2012-02-14 13:47:18 CET