Re: [BUG FIX] Uninitialized var fargtypes used.

At Mon, 11 Nov 2019 12:32:38 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
> Ranier Vilela <ranier_gyn@hotmail.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