From: Jeff King <peff@peff.net>
Date: Thu, 26 Mar 2026 17:34:02 GMT
Subject: Re: [PATCH 1/6] do not discard const: the simple cases
Message-ID: <20260326173402.GB2447148@coredump.intra.peff.net>
In-Reply-To: <a3a1d2759a0ec5a3ee285689832832e5e3a63768.1774537954.git.git@grubix.eu>

On Thu, Mar 26, 2026 at 04:22:47PM +0100, Michael J Gruber wrote:

> This patch covers the easy cases where we deal with a non-const pointer
> to begin with. It is solved by the cast `bar = (char *) foo`.

I think we can often do better, though. For example, in this case:

> diff --git a/builtin/config.c b/builtin/config.c
> index 7c4857be62..bd277e5911 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -852,7 +852,7 @@ static int get_urlmatch(const struct config_location_options *opts,
>  		die("%s", config.url.err);
>  
>  	config.section = xstrdup_tolower(var);
> -	section_tail = strchr(config.section, '.');
> +	section_tail = (char *) strchr(config.section, '.');
>  	if (section_tail) {
>  		*section_tail = '\0';
>  		config.key = section_tail + 1;

We know that it is OK to cast away the const-ness because config.section
is writeable, which we know because it just came from xstrdup(). So why
is it const in the first place? Because the pointer is in a struct which
may be used with other const strings.

But we can untangle this for the compiler without having to cast by
using a non-const alias, like:

  char *section;
  ...
  config.section = section = xstrdup_tolower(var);
  section_tail = strchr(section, '.');

Which I think is safer and shows the intent more clearly.

Some of the other cases below can use similar techniques (e.g., I think
packet_reader's line probably ought to be non-const).

-Peff