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
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 8e926539e6..1686a80403 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2035,9 +2035,6 @@ LookupFuncNameInternal(List *funcname, int nargs, const Oid *argtypes,
 {
     FuncCandidateList clist;
 
-    /* Passing NULL for argtypes is no longer allowed */
-    Assert(argtypes);
-
     /* Always set *lookupError, to forestall uninitialized-variable warnings */
     *lookupError = FUNCLOOKUP_NOSUCHFUNC;
 
@@ -2047,7 +2044,7 @@ LookupFuncNameInternal(List *funcname, int nargs, const Oid *argtypes,
     /*
      * If no arguments were specified, the name must yield a unique candidate.
      */
-    if (nargs < 0)
+    if (nargs <= 0)
     {
         if (clist)
         {
@@ -2064,6 +2061,9 @@ LookupFuncNameInternal(List *funcname, int nargs, const Oid *argtypes,
             return InvalidOid;
     }
 
+    /* Passing NULL for argtypes is no longer allowed */
+    Assert(argtypes);
+
     /*
      * Otherwise, look for a match to the arg types.  FuncnameGetCandidates
      * has ensured that there's at most one match in the returned list.