From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 26 Mar 2026 17:45:41 GMT
Subject: Re: [PATCH 1/6] do not discard const: the simple cases
Message-ID: <xmqqy0jepjqy.fsf@gitster.g>
In-Reply-To: <20260326173402.GB2447148@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> 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.

Yeah, this is much clearer.