Re: GetRelationPublicationActions. - Remove unreachable code

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: GetRelationPublicationActions. - Remove unreachable code
Date: 2022-02-10 01:37:30
Message-ID: CAHut+PsNbE21RpBV0Xdpu9o203bjZ+M2yge563-Pef4Ga2kzPw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Feb 10, 2022 at 11:34 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Peter Smith <smithpb2250(at)gmail(dot)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

Attachment Content-Type Size
v2-0001-GetRelationPublicationActions-comment-code.patch application/octet-stream 911 bytes

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-02-10 01:47:39 Re: make MaxBackends available in _PG_init
Previous Message Kyotaro Horiguchi 2022-02-10 01:18:15 Re: Possible uninitialized use of the variables (src/backend/access/transam/twophase.c)