Re: BUG #18845: DEREF_OF_NULL.RET guc_malloc possibly returns NULL

> On 15 Mar 2025, at 18:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Daniel Gustafsson <daniel@yesql.se> writes:
>> On 14 Mar 2025, at 15:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Indeed.  Also, a quick survey shows a lot of inconsistency in
>>> guc_malloc callers --- some are lazy and just use ERROR rather
>>> than LOG-and-return.  That's probably all right for PGC_POSTMASTER
>>> variables (since there's no chance of continuing anyway) but
>>> perhaps it's worth improving elsewhere.
>
>> Turns out there was one more guc_malloc(LOG..  which didn't inspect the
>> returned allocation in check_synchronized_standby_slots.  On top of that there
>> were a few non PGC_POSTMASTER check functions that could return false and let
>> the GUC machinery handle it if we want to be consistent.
>
> Patch looks good as far as it goes, but I nosed around and found
> a few other things:
>
> * After sleeping on it, I think we ought to change the
> guc_malloc(ERROR,...) calls in the PGC_POSTMASTER cases too.
> While this won't have any functional effect, what it does do
> is remove examples of bad practice that are likely to get
> copied-and-pasted to somewhere where it matters.  The affected
> functions seem to be
> check_recovery_target_lsn
> check_recovery_target_timeline
> check_recovery_target_xid
> check_debug_io_direct
>
> * check_application_name and check_cluster_name are using WARNING
> not LOG in their guc_strdup calls.  That seems to have been
> decided by flipping a coin; let's sync them with everyplace else.
>
> * init_custom_variable uses ERROR in a guc_malloc and a guc_strdup
> call.  These should probably be changed to FATAL, on the same grounds
> that the error levels earlier in that function are FATAL: we are
> partway through initializing a newly-loaded extension, so it's
> unlikely that trying to continue the session is going to end well.

Those are all good points, I initially didn't think we should touch the
PGC_POSTMASTER cases but you are correct that avoiding back copy pastes to
happen is a Good Thing. The attached has all these fixes added.

--
Daniel Gustafsson