Re: [PATCH v3 2/6] string-list: introduce `string_list_setlen()`

From: Taylor Blau <me@ttaylorr.com>
Date: Apr 25, 2023, 21:00 UTC
Message-ID: <ZEg/XPbtaAx0tPzK@nand.local>
In-Reply-To: <20230425062107.GA4061254@coredump.intra.peff.net>

On Tue, Apr 25, 2023 at 02:21:07AM -0400, Jeff King wrote:

Show 10 quoted lines

> I think we _could_ do something like:
>
>   for (i = nr; i < list->nr; i++) {
> 	if (list->items[i].util)
> 		BUG("truncated string list item has non-NULL util field");
>   }
>
> though that is technically tighter than we need to be (it could be an
> unowned util field, after all; we don't know what it means here). So I'm
> inclined to leave your patch as-is.

I think there are two ways to do it, either:

  - something like what you wrote above, perhaps with an additional
    `free_util` bit on the string_list itself (which might make it
    convenient to drop all of the `free_util` parameters that permeate
    its API)

  - have string_list_setlen() take a `free_util` argument itself, in
    which case your code would change to:

    if (free_util) {
      for (i = nr; i < list->nr; i++)
        free(list->items[i].util)
    }

Show 5 quoted lines

> This would all be easier if the string_list had a field for "we own the
> util fields, too" just like it has strdup_strings. Or even a free-ing
> function. But instead we have ad-hoc solutions like "free_util" and
> string_list_clear_func(). But that's really outside the scope of your
> series. </rant> :)

;-).

Thanks, Taylor