On Thu, Feb 10, 2022 at 11:34 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Peter Smith <smithpb2250@gmail.com> writes:
> > There appears to be some unreachable code in the relcache function
> > GetRelationPublicationActions.
> > When the 'relation->rd_pubactions' is not NULL then the function
> > unconditionally returns early (near the top).
> > Therefore, the following code (near the bottom) seems to have no
> > purpose because IIUC the 'rd_pubactions' can never be not NULL here.
>
> I'm not sure it's as unreachable as all that. What you're not
> accounting for is the possibility of recursive cache loading,
> ie something down inside the catalog fetches we have to do here
> could already have made the field valid.
>
> Admittedly, that's probably not very likely given expected usage
> patterns (to wit, that system catalogs shouldn't really have
> publication actions). But there are other places in relcache.c where
> a coding pattern similar to this is demonstrably necessary to avoid
> memory leakage. I'd rather leave the code as-is, maybe add a comment
> along the lines of "rd_pubactions is probably still NULL, but just in
> case something already made it valid, avoid leaking memory".
OK. Thanks for your explanation and advice.
PSA another patch to just a comment as suggested.
------
Kind Regards,
Peter Smith.
Fujitsu Australia
