On Tue, Jun 06, 2023 at 09:00:58AM +0200, Patrick Steinhardt wrote: > 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
