Re: [PATCH v2 11/16] revision.h: store hidden refs in a `strvec`
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Jun 20, 2023, 12:16 UTC
- Message-ID
- <ZJGYs4hWjdMTSMm4@nand.local>
- In-Reply-To
- <ZH7ZqjYqyvSOHY8u@ncase>
On Tue, Jun 06, 2023 at 09:00:58AM +0200, Patrick Steinhardt wrote:
Show 26 quoted lines
> On Mon, May 15, 2023 at 03:23:39PM -0400, Taylor Blau wrote: > > In subsequent commits, it will be convenient to have a 'const char **' > > of hidden refs (matching `transfer.hiderefs`, `uploadpack.hideRefs`, > > etc.), instead of a `string_list`. > > > > Convert spots throughout the tree that store the list of hidden refs > > from a `string_list` to a `strvec`. > > > > Note that in `parse_hide_refs_config()` there is an ugly const-cast used > > to avoid an extra copy of each value before trimming any trailing slash > > characters. This could instead be written as: > > > > ref = xstrdup(value); > > len = strlen(ref); > > while (len && ref[len - 1] == '/') > > ref[--len] = '\0'; > > strvec_push(hide_refs, ref); > > free(ref); > > > > but the double-copy (once when calling `xstrdup()`, and another via > > `strvec_push()`) is wasteful. > > I guess the proper way to fix this would be to introduce something like > a `strvec_push_nodup()` function that takes ownership. And in fact this > helper exists already, but it's declared as static. So we could get > around the ugly cast with a simple change to expose the helper function.
We could, but I'd prefer to explore doing so outside of this series, since I think strvec_push_nodup() is a little bit of a footgun unless you are thinking carefully about ownership. So making the function part of the exposed API may be controversial.
Thanks, Taylor