Re: [PATCH 16/20] strvec: add functions to replace and remove strings
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 24, 2024, 06:56 UTC
- Message-ID
- <ZlA6GkowIo16qprV@tanuki>
- In-Reply-To
- <CAPig+cTKrUL1AzY4Y+M130AvH5S=kSTdwo542T_GdFC=6JUu=Q@mail.gmail.com>
On Thu, May 23, 2024 at 01:09:39PM -0400, Eric Sunshine wrote:
Show 23 quoted lines
> On Thu, May 23, 2024 at 8:27 AM Patrick Steinhardt <ps@pks.im> wrote:
> > Add two functions that allow to replace and remove strings contained in
> > the strvec. This will be used by a subsequent commit that refactors
> > git-mv(1).
> > [...]
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/strvec.c b/strvec.c
> > @@ -56,6 +56,26 @@ void strvec_pushv(struct strvec *array, const char **items)
> > +const char *strvec_replace(struct strvec *array, size_t idx, const char *replacement)
> > +{
> > + char *to_free;
> > + if (idx >= array->nr)
> > + BUG("index outside of array boundary");
> > + to_free = (char *) array->v[idx];
> > + array->v[idx] = xstrdup(replacement);
> > + free(to_free);
> > + return array->v[idx];
> > +}
>
> The reason you delay calling free() until after xstrdup() is to
> protect against the case when `replacement` is a substring of the
> string already stored at `v[idx]`, correct?Yup. The patches for the strvec API have been lying around for quite a while as I have originally implemented them in a different context. And there I did hit this edge case indeed. It's rather unlikely to happen overall, but I think it shouldn't hurt to be prepared.
There is no coverage of this scenario in this patch series though. Let me add a unit test for this.
Patrick