Re: Valgrind - showing memory leaks

On 2025-May-08, Tom Lane wrote:

> Uh ... yeah it is, down at the bottom of the function:
> 
>     /* Install array only after it's fully valid */
>     relation->rd_att->constr->check = check;
>     relation->rd_att->constr->num_check = found;
> 
> So it seems like valgrind is wrong here, or else we're leaking the
> whole rd_att structure later on somehow.

Well, the problem is that if num_check is zero, FreeTupleDesc() doesn't
free ->check.

> In any case, you're right that asking for a zero-size chunk is pretty
> pointless.  I'd support doing
> 
> +    if (ncheck > 0)
> +        check = (ConstrCheck *)
> +            MemoryContextAllocZero(CacheMemoryContext,
> +                                   ncheck * sizeof(ConstrCheck));
> +    else
> +        check = NULL;
> 
> but I think we have to make sure it's null if we don't palloc it.

Done that way, thanks.

> > On the other hand, the bug I was thinking about, is that if the table
> > has an invalid not-null constraint, we leak during detoasting in
> > extractNotNullColumn().  [...] But it's no longer so obvious that
> > extractNotNullColumn is okay to leak those few bytes.
> 
> Given your description it still sounds fine to me.

Cool, I left it alone.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
“Cuando no hay humildad las personas se degradan” (A. Christie)