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

[Hou, Zhijie]

Hi Ishii san,

> It is possible that Coverity was wrong (false positive) but can you please take a look at this?

No problem. I'll look into it

Best regards,
houzj

-----Original Message-----
From: Tatsuo Ishii [mailto:ishii at sraoss.co.jp] 
Sent: Monday, July 27, 2020 9:30 AM
To: Hou, Zhijie/侯 志杰 <houzj.fnst at cn.fujitsu.com>
Cc: pgpool-hackers at pgpool.net
Subject: Re: [pgpool-hackers: 3738] Re: problem when getting function name

Hi Hou, Zhijie,

>>>>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?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp