On Mon, Mar 2, 2026 at 11:34 PM Branko Čibej <[email protected]> wrote:
> On 2. 3. 26 21:17, Timofei Zhakov wrote:
>
> I'm pretty sure that it's a new feature in the compiler that detects if
> the result of a function returning a pointer to a string was saved into
> non-constant pointer from const pointer. This seems reasonable and it seems
> like consts were misused in those places.
>
> [[[
> [39/469] Building C object
> CMakeFiles/libsvn_subr.dir/subversion/libsvn_subr/gpg_agent.c.o
> /home/tima/dev/svn-trunk/subversion/libsvn_subr/gpg_agent.c: In function
> ‘find_running_gpg_agent’:
> /home/tima/dev/svn-trunk/subversion/libsvn_subr/gpg_agent.c:391:6:
> warning: assignment discards ‘const’ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
> 391 | ep = strchr(p, '\n');
>
>
> But then in your patch:
>
> --- subversion/libsvn_subr/gpg_agent.c (revision 1932024)
> +++ subversion/libsvn_subr/gpg_agent.c (working copy)
> @@ -326,7 +326,7 @@ find_running_gpg_agent(int *new_sd, apr_pool_t *po
> char *buffer;
> const char *socket_name = find_gpg_agent_socket(pool, pool);
> const char *request = NULL;
> - const char *p = NULL;
> + char *p = NULL;
> char *ep = NULL;
> int sd;
>
>
> This makes no sense. Not only is 'p' already const, nothing is being
> modified through '*p' but something _is_ being modified through '*ep'.
>
Later in the code:
[[[
ep = strchr(p, '\n');
if (ep != NULL)
*ep = '\0';
]]]
'ep' points to the same memory as 'p' since strchr essentially takes the
pointer and moves it. So for it to modify the string (assgning *ep to \0),
ep needs to be mutable hence does p.
>
>
> [226/469] Building C object
> CMakeFiles/libsvn_diff.dir/subversion/libsvn_diff/parse-diff.c.o
> /home/tima/dev/svn-trunk/subversion/libsvn_diff/parse-diff.c: In function
> ‘git_start’:
> /home/tima/dev/svn-trunk/subversion/libsvn_diff/parse-diff.c:1582:19:
> warning: assignment discards ‘const’ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
> 1582 | new_path_marker = strstr(old_path_marker, " b/");
> | ^
> /home/tima/dev/svn-trunk/subversion/libsvn_diff/parse-diff.c:1610:23:
> warning: assignment discards ‘const’ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
> 1610 | new_path_marker = strstr(new_path_start, " b/");
> | ^
> ]]]
>
>
> Same here, your patch does this:
>
> --- subversion/libsvn_diff/parse-diff.c (revision 1932024)
> +++ subversion/libsvn_diff/parse-diff.c (working copy)
> @@ -1545,12 +1545,12 @@ static svn_error_t *
> git_start(enum parse_state *new_state, char *line, svn_patch_t *patch,
> apr_pool_t *result_pool, apr_pool_t *scratch_pool)
> {
> - const char *old_path_start;
> + char *old_path_start;
> char *old_path_end;
> - const char *new_path_start;
> - const char *new_path_end;
> + char *new_path_start;
> + char *new_path_end;
> char *new_path_marker;
> - const char *old_path_marker;
> + char *old_path_marker;
>
>
> Again, removing the 'const's makes no sense.
>
>
Here mutability needs to be promoted by all of these pointers to the memory
being modified. Actually I did a double check and new_path_end can remain
constant. The only mutation happens here:
[[[
/* Are the paths before and after the " b/" marker the same? */
if (len_old == len_new
&& ! strncmp(old_path_start, new_path_start, len_old))
{
*old_path_end = '\0';
SVN_ERR(grab_filename(&patch->old_filename, old_path_start,
result_pool, scratch_pool));
SVN_ERR(grab_filename(&patch->new_filename, new_path_start,
result_pool, scratch_pool));
break;
}
]]]
> I prepared a patch that fixes them by adding const where needed. Can
> anyone correct me if I'm misunderstanding something?
>
>
>
> There is something fishy going on here. For example, this is the prototype
> for strstr:
>
> char *strstr(const char*, const char*);
>
>
> and if you look at this bit of code in your error report:
>
> new_path_marker = strstr(new_path_start, " b/");
>
>
> new_path_start is a 'const char*', " b/" is a 'char*' (in C, but a 'const
> char*' in C++), and new_path_marker is a 'const char*'. In the case of both
> the second parameter and the return value, we're assigning a 'char*' to a
> 'const char*', this conversion is implicit and safe and there should be no
> error reported here.
>
> The warning message makes no sense at all.
>
> -- Brane
>
>
--
Timofei Zhakov
