arthur.j.odwyer@gmail.com writes:
> When MALLOC fails, pg_regcomp leaks memory in at least two places:
> (A) In freev(), the line
> freesubre(info, v, v->tree);
> should be
> freesubre(info, NULL, v->tree);
> as otherwise the "freed" subres will end up on v->treefree, which is leaked
> by the cleanst() two lines later.
Hmm ... what version of the code are you looking at exactly? There's no
"info" argument here in Postgres.
> That is, given the precondition that there are things in v->tree that aren't
> in v->treechain.
The problem with this proposal is that if there are subres in v->tree
that *are* in the treechain, we'll possibly try to free them twice
(if they're not marked INUSE), and definitely will be accessing
already-freed memory when cleanst looks at them.
It looks to me like there are two different operating regimes in this
code: one where all the subres are still in the treechain, and one where
we've marked as INUSE all the ones that are reachable from v->tree and
garbage-collected the rest. The markst/cleanst steps in pg_regcomp are
where the conversion is made. freev needs to work correctly either
before or after that.
In short, I think you're right that there's a bug here, but this is
not a good fix for it. I'm not sure freev has enough info to do the
right thing; we may need to rethink the data structure invariants,
so that there's not two different ways to clean up.
> (B) newlacon() leaks memory if REALLOC returns NULL on this line:
> v->lacons = (struct subre *) REALLOC(v->lacons,
> (v->nlacons + 1) * sizeof(struct subre));
> The fix is to use the same idiom already used everywhere else REALLOC is
> called in this module.
A temp variable, you mean. Yeah, that's a bug.
regards, tom lane