Re: [PATCH 1/6] do not discard const: the simple cases

From: Jeff King <peff@peff.net>
Date: Mar 26, 2026, 17:34 UTC
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:

Show 13 quoted lines

> 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