Re: [BUG FIX] Uninitialized var fargtypes used.

At Mon, 11 Nov 2019 12:32:38 -0500, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
> Ranier Vilela <ranier_gyn(at)hotmail(dot)com> writes:
> > Can anyone check this bug fix?
>
> > - Oid fargtypes[1]; /* dummy */
> > + Oid fargtypes[1] = {InvalidOid, InvalidOid}; /* dummy */
>
> Well, it's wrong on its face, because that array only has one element
> not two. But why do you care? The element will never be accessed.
>
> The only reason we declare this variable at all is that LookupFuncName
> requires a non-null pointer, which if memory serves is because memcmp()
> with a null pointer is formally undefined even if the count is zero,
> cf commit 0a52d378b.

Yes, what is needed there is a valid pointer with any content.

> Maybe it would've been better to make LookupFuncName deal with the
> case instead of requiring callers to do strange things. But I don't
> see any bug here.

Actually it's not an actual bug but a cosmetic adjustment, but not
that bad, I think. Requiring dummy pointer is already a strange thing.

By the way looking closer the function, IIUC, very small change can do
that.

> /*
> * If no arguments were specified, the name must yield a unique candidate.
> */
> if (nargs < 0)
> {

We can change the condition with "nargs <= 0" and it should return the
only element in clist. If the catalog is broken we may get
FUNCLOOKUP_AMBIGUOUS but it seems rather the correct behavior.

This allows argtypes == NULL and makes the caller-side tweak useless.

Thoughts?

--
Kyotaro Horiguchi
NTT Open Source Software Center