AlterSubscription_refresh "wrconn" wrong variable?

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: AlterSubscription_refresh "wrconn" wrong variable?
Date: 2021-05-03 23:29:42
Message-ID: CAHut+Pu7Jv9L2BOEx_Z0UtJxfDevQSAUW2mJqWU+CtmDrEZVAg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

While reviewing some logical replication code I stumbled across a
variable usage that looks suspicious to me.

Note that the AlterSubscription_refresh function (unlike other
functions in the subscriptioncmds.c) is using the global variable
"wrconn" instead of a local stack variable of the same name. I was
unable to think of any good reason why it would be deliberately doing
this, so my guess is that it is simply an accidental mistake that has
gone unnoticed because the compiler was silently equally happy just
using the global var.

Apparently, this is not causing any reported problems because it seems
like the code has been this way for ~4 years [1].

Even so, it doesn't look intentional to me and I felt that there may
be unknown consequences (e.g. resource leakage?) of just blatting over
the global var. So, PSA a small patch to make this
AlterSubscription_refresh function use a stack variable consistent
with the other nearby functions.

Thoughts?

------
[1] https://github.com/postgres/postgres/commit/7c4f52409a8c7d85ed169bbbc1f6092274d03920#

Kind Regards,
Peter Smith.
Fujitsu Australia

Attachment Content-Type Size
v1-0001-Fix-wrconn.-Use-stack-variable.patch application/octet-stream 1.8 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2021-05-04 00:15:04 Re: MaxOffsetNumber for Table AMs
Previous Message David Rowley 2021-05-03 23:02:22 Re: Performance Evaluation of Result Cache by using TPC-DS