Re: [PATCH 04/12] pager: explicitly cast away strchr() constness

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> When we do:
>
>   char *cp = strchr(argv[i], '=');
>
> it implicitly removes the constness from argv[i]. We need "cp" to remain
> writable (since we overwrite it with a NUL). In theory we should be able
> to drop the const from argv[i], because it is a sub-pointer into our
> duplicated pager_env variable.
>
> But we get it from split_cmdline(), which uses the traditional "const
> char **" type for argv. This is overly limiting, but changing it would
> be awkward for all the other callers of split_cmdline().

Yeah, it was the first thing that came to my mind that const char
**argv is the source of the problem.  We could cast the pointer we
give to split_cmdline() and drop const from argv[] instead, which
may make the in-code comment unnecessary but the patch we see here
is good enough.



>
> Let's do an explicit cast with a note about why it is OK. This is enough
> to silence compiler warnings about the implicit const problems.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  pager.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/pager.c b/pager.c
> index 5531fff50e..801ba392f2 100644
> --- a/pager.c
> +++ b/pager.c
> @@ -118,7 +118,8 @@ static void setup_pager_env(struct strvec *env)
>  			split_cmdline_strerror(n));
>  
>  	for (i = 0; i < n; i++) {
> -		char *cp = strchr(argv[i], '=');
> +		/* we know this is writable because it was split from pager_env */
> +		char *cp = strchr((char *)argv[i], '=');
>  
>  		if (!cp)
>  			die("malformed build-time PAGER_ENV");