On Wed, Aug 07, 2024 at 07:25:51PM +1000, James Liu wrote:
> > Refactor the code such that we have two pointers for each of these
> > strings: one that holds the value as accessed by other subsystems, and
> > one that points to the same string in case it has been allocated. Like
> > this, we can safely free the second pointer and thus plug those memory
> > leaks.
> >
> > diff --git a/userdiff.c b/userdiff.c
> > index c4ebb9ff73..989629149f 100644
> > --- a/userdiff.c
> > +++ b/userdiff.c
> > @@ -399,8 +399,11 @@ static struct userdiff_driver *userdiff_find_by_namelen(const char *name, size_t
> > static int parse_funcname(struct userdiff_funcname *f, const char *k,
> > const char *v, int cflags)
> > {
> > - if (git_config_string((char **) &f->pattern, k, v) < 0)
> > + f->pattern = NULL;
> > + FREE_AND_NULL(f->pattern_owned);
> > + if (git_config_string(&f->pattern_owned, k, v) < 0)
> > return -1;
> > + f->pattern = f->pattern_owned;
> > f->cflags = cflags;
> > return 0;
> > }
>
> I'm not sure if I understand this change completely. We don't seem to be
> using `pattern_owned` (and the other *_owned) fields differently from
> their regular counterparts.
>
> Is it because we can't do the following?
>
> FREE_AND_NULL((char **)f->pattern);
Yup. We have a bunch of statically defined userdiff drivers, all of
which use string constants as patterns. We thus cannot reliably free
those and instead have to track the allocated strings in a separate
variable.
Patrick
Attachment:
signature.asc
Description: PGP signature
