Re: [PATCH 04/10] string-list API: mark "struct_string_list" to "for_each_string_list" const

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 27, 2022, 23:04 UTC
Message-ID: <221028.86a65gam0o.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqlep1kqpy.fsf@gitster.g>

On Thu, Oct 27 2022, Junio C Hamano wrote:

Show 18 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Add a "const" to the "struct string_list *" passed to
>> for_each_string_list().
>>
>> This is arguably abuse of the type system, as the
>> "string_list_each_func_t fn" take a "struct string_list_item *",
>> i.e. not one with a "const", and those functions *can* modify those
>> items.
>>
>> But as we'll see in a subsequent commit we have other such iteration
>> functions that could benefit from a "const", i.e. to declare that
>> we're not altering the list itself, even though we might be calling
>> functions that alter its values.
>
> The callback functions are allowed to (by taking a non-const
> pointer) modify the items, but are there ones that actually modify
> them?

Tree-wide that's:

	 11 files changed, 18 insertions(+), 18 deletions(-)

I.e. a bunch of changes like:

	-static int get_notes_refs(struct string_list_item *item, void *arg)
	+static int get_notes_refs(const struct string_list_item *item, void *arg)

It turns out there's a grand total of one user of that:
	
	setup.c: In function ‘canonicalize_ceiling_entry’:
	setup.c:1102:30: error: assignment of member ‘string’ in read-only object
	 1102 |                 item->string = real_path;
	      |                              ^

But note that that's for the "filter" variant. In any case using the same function pointer type in eb5f0c7a616 (string_list: add a new function, filter_string_list(), 2012-09-12) for both was probably a mistake.

But still, I think it's best not to do anything about *that*. I.e. it makes sense for such an interface to say that the iterator helper takes your const list, i.e. unlike filter_string_list() it's not expected to be changing the list itself.

But you as as the caller are then free to change list items you're given.