On Tue, Mar 3, 2026 at 4:51 PM Branko Čibej <[email protected]> wrote:
> On 3. 3. 26 14:55, Evgeny Kotkov via dev wrote: > > Timofei Zhakov <[email protected]> <[email protected]> writes: > > > I prepared a patch that fixes them by adding const where needed. Can anyone > correct me if I'm misunderstanding something? > > I think that the patch indeed improves the situation by propagating the non- > const types upwards, so that everything happens without implicit const > discards. So +1 in general. > > > > I'll note that changes like this one: > > --- subversion/libsvn_subr/version.c (revision 1932024) > +++ subversion/libsvn_subr/version.c (working copy) > @@ -253,7 +253,7 @@ svn_version__parse_version_string(svn_version_t ** > require that it be present. */ > if (pieces->nelts == 3) > { > - const char *piece = APR_ARRAY_IDX(pieces, 2, const char *); > + char *piece = APR_ARRAY_IDX(pieces, 2, char *); > char *hyphen = strchr(piece, '-'); > if (hyphen) > { > > > have much wider consequences than this one-line change. The 'pieces' come > from 'svn_cstring_split()' which is documented as > > /** Divide @a input into substrings, interpreting any char from @a sep > * as a token separator. > * > * Return an array of copies of those substrings (plain const char*), > * allocating both the array and the copies in @a pool. > > > and this is a public API. So, strictly speaking, the proposed change is an > API violation, as opposed to what we were doing before with is "just" a > standard C hack. > It is not a hack because the docstring explicitly states that the result would contain "copies" of those substrings. Which technically means that we are free to do whatever we want with this memory. I think in this exact example the docstring of svn_cstring_split itself is slightly misleading, because the elements in the result array can be treated as 'char *' as well. > > If we want to be really sure we're doing everything right then in all the > cases where we get a 'const char*' from anywhere and then change it, we > should create a local copy first. I mean, who knows if it isn't pointing to > a static string in write-protected memory? > > Or we could avoid code churn by assuming that whoever is reading the code > understands C90 semantics. > If a string is received as an argument of a function and is marked as 'const char *', it could be in write-protected memory which libsvn should be able to deal with. It should be safe for users to assume that it never changes. That is why the const modifier exists in the first place. It gives some guarantees about the API. -- Timofei Zhakov
