On Fri, Oct 10, 2025 at 02:14:49AM -0400, Jeff King wrote:

On Tue, Oct 07, 2025 at 04:26:26PM -0400, Taylor Blau wrote:

quoted

quoted

quoted

+	for_each_string_list_item(item, &names)
+		generated_pack_install((struct generated_pack *)item->util,

This cast should be unnecessary, right? `item->util` is a void pointer,
so C should do the cast implicitly.

It's unnecessary, but I dislike implicit casts from 'void*' to any other
type. This makes it clearer how we're supposed to interpret the value in
item->util, but I'm happy to change it to use the implicit cast if you
feel strongly about it.

I tend to avoid casts when possible, because they can mask unexpected
conversions. E.g., if item->util's type changed, your cast means we
would never notice it.

But that cuts both ways. If the prototype for generated_pack_install()
changes, it is only your cast that would tell the compiler that this
caller needs to be updated. Ultimately it is the spot that _sets_
item->util that needs to change, but hopefully flagging this spot would
point us in the right direction.

And that does seem more likely than item->util changing away from void.
(To be clear, I don't think either is that likely, I'm just musing on
whether casts like this are helpful in general).

OK, that's a very compelling argument. I'll drop the cast on my local
version.

(As a side note, it doesn't look like we mention this convention in our
CodingGuidelines, and we probably should, including the rationale that
you gave above here. That seems like another good candidate for
#leftoverbits.)

Thanks,
Taylor