Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c)

On Wed, Jun 05, 2024 at 01:12:41PM +0900, Kyotaro Horiguchi wrote:
> At Mon, 27 May 2024 11:31:24 -0300, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote in
>> The function *plpgsql_inline_handler* can use uninitialized
>> variable retval, if PG_TRY fails.
>> Fix like function*plpgsql_call_handler* wich declare retval as
>> volatile and initialize to (Datum 0).

You forgot to read elog.h, explaining under which circumstances
variables related to TRY/CATCH block should be marked as volatile.
There is a big "Note:" paragraph.

It is not the first time that this is mentioned on this list: but
sending a report without looking at the reason why a change is
justified makes everybody waste time. That's not productive.

> If PG_TRY fails, retval is not actually accessed, so no real issue
> exists. Commit 7292fd8f1c changed plpgsql_call_handler() to the
> current form, but as stated in its commit message, it did not fix a
> real issue and was solely to silence compiler.

This complain was from lapwing, that uses a version of gcc which
produces a lot of noise with incorrect issues. It is one of the only
32b buildfarm members, so it still has a lot of value.

> I believe we do not need to modify plpgsql_inline_handler() unless
> compiler actually issues a false warning for it.

If we were to do something, that would be to remove the volatile from
plpgsql_call_handler() at the end once we don't have in the buildfarm
compilers that complain about it, because there is no reason to use a
volatile in this case. :)
--
Michael