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

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

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.