On 3. 3. 26 14:55, Evgeny Kotkov via dev wrote:
Timofei Zhakov<[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.
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.
-- Brane
