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
