On Tue, May 04, 2021 at 10:35:02PM -0400, Tom Lane wrote:
> Peter Smith <smithpb2...@gmail.com> writes:
> > This patch replaces the global "wrconn" in AlterSubscription_refresh with a 
> > local variable of the same name, making it consistent with other functions 
> > in subscriptioncmds.c (e.g. DropSubscription).
> > The global wrconn is only meant to be used for logical apply/tablesync 
> > worker.
> > Using the global/incorrect wrconn in AlterSubscription_refresh doesn't 
> > normally cause any problems, but harm is still posslble if the apply worker 
> > ever manages to do a subscription refresh. e.g. see [1]
> 
> Hm.  I would actually place the blame for this on whoever thought
> it was okay to name a global variable something as generic as
> "wrconn".   Let's rename that while we're at it, say to something
> like "tablesync_wrconn" (feel free to bikeshed).

Yea, I think global vars should have at least 1) an underscore, or 2) a
capital, and in any case be 3) longer than 6 chars.

There's very few which violate both "arms" of that rule - should anything else
be renamed, too ?

$ git grep -E '^static [^(=]*\<[[:lower:]]{,6}(;$| =)' src/backend/'*.c'
src/backend/access/heap/vacuumlazy.c:static int elevel = -1;
src/backend/access/transam/xloginsert.c:static XLogRecData *rdatas;
src/backend/bootstrap/bootstrap.c:static MemoryContext nogc = NULL;     /* 
special no-gc mem context */
src/backend/libpq/be-fsstubs.c:static MemoryContext fscxt = NULL;
src/backend/replication/walreceiver.c:static WalReceiverConn *wrconn = NULL;
src/backend/replication/walsender.c:static StringInfoData tmpbuf;
src/backend/storage/file/fd.c:static int        nfile = 0;
src/backend/utils/misc/sampling.c:static ReservoirStateData oldrs;

pryzbyj@pryzbyj:~/src/postgres$ git grep -lE '^static 
[^(=]*\<[[:lower:]]{,6}(;$| =)' src/backend/'*.c' |xargs wc -l |sort -nr
  4326 src/backend/access/heap/vacuumlazy.c
  3781 src/backend/storage/file/fd.c
  3698 src/backend/replication/walsender.c
  1428 src/backend/replication/walreceiver.c
  1227 src/backend/access/transam/xloginsert.c
  1155 src/backend/bootstrap/bootstrap.c
   864 src/backend/libpq/be-fsstubs.c
   296 src/backend/utils/misc/sampling.c

-- 
Justin


Reply via email to