[pgpool-hackers: 3742] Re: problem when getting function name

[Hou, Zhijie]

Hi Ishii san,

> >>>>Looks good. If we want to be more rigorous, we should consider the
> function args but in practice no one wants to mix immutable and
> non->immutable functions having different arguments (overloading). So I
> think this is enough.
> >>>
> >>> I tried to split "schema.funcname"  and put them into sql here, but
> I found the function "remove_quotes_and_schema_from_relname(char *)" can
> not get correct functionname, when schemaname include dots  (such as
> "schema.name").
> >>>
> >>> So I try to copy postgres's function 'SplitIdentifierString' which can
> split str correctly, and I let remove_quotes_and_schema_from_relname to
> call SplitIdentifierString inside it.
> >>>
> >>> Besides, I add some regression test in 006-memcache for function check,
> and made a patch.
> >>
> >> Thanks. I will look into this.
> >
> > Your patch has been committed with minor comment fix. Thank you!
> 
> Today I ran Coverity against master branch and it complains following which
> could relate to your patch:
> 
> ** CID 1430580:  Null pointer dereferences  (REVERSE_INULL)
> /src/utils/pool_select_walker.c: 1050 in is_immutable_function()
> 
> 
> ______________________________________________________________________
> __________________________________
> *** CID 1430580:  Null pointer dereferences  (REVERSE_INULL)
> /src/utils/pool_select_walker.c: 1050 in is_immutable_function()
> 1044     	rawstring = pstrdup(fname);
> 1045
> 1046     	/* split "schemaname.funcname" */
> 1047     	if(!SplitIdentifierString(rawstring, '.', (Node **)
> &names) ||
> 1048     		names == NIL)
> 1049     	{
> >>>     CID 1430580:  Null pointer dereferences  (REVERSE_INULL)
> >>>     Null-checking "rawstring" suggests that it may be null, but it has
> already been dereferenced on all paths leading to the check.
> 1050     		if(rawstring)
> 1051     			pfree(rawstring);
> 1052     		if(names)
> 1053     			list_free(names);
> 1054
> 1055     		ereport(WARNING,
> 
> It is possible that Coverity was wrong (false positive) but can you please
> take a look at this?


Since SplitIdentifierString function does not free rawstring, and rawstring seems will not be NULL in current code.
So I think it will not cause Null pointer dereferences.

However, the 'if(rawstring)' is a redundant condition.
we can remove it and may be Coverity will not complain again.

Best regards,
houzj
 



-------------- next part --------------
A non-text attachment was scrubbed...
Name: Update-pool_select_walker.c.patch
Type: application/octet-stream
Size: 846 bytes
Desc: Update-pool_select_walker.c.patch
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200727/85c878df/attachment.obj>