Nitpick/question: Use of aliases for global variables in functions

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Nitpick/question: Use of aliases for global variables in functions
Date: 2021-08-20 02:57:20
Message-ID: CADkLM=cP_L98m_yCiSanRFSr2GoHkvPgUVzm2S5GFDb0w8b6Sw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I'm using an ongoing patch review to educate myself on parts of the
codebase.

In src/backend/access/transam/xact.c, I'm noticing a code style
inconsistency.

In some cases, a function will declare a variable of some struct pointer
type, assign it to a globally declared struct, and then use it like this:

bool
IsTransactionState(void)
{
TransactionState s = CurrentTransactionState;

/*
* TRANS_DEFAULT and TRANS_ABORT are obviously unsafe states. However,
we
* also reject the startup/shutdown states TRANS_START, TRANS_COMMIT,
* TRANS_PREPARE since it might be too soon or too late within those
* transition states to do anything interesting. Hence, the only
"valid"
* state is TRANS_INPROGRESS.
*/
return (s->state == TRANS_INPROGRESS);
}

And in other cases, the function will just reference the global directly

bool
TransactionStartedDuringRecovery(void)
{
return CurrentTransactionState->startedInRecovery;
}

Clearly, this isn't a big deal. I'm willing to bet that the compiler looks
at the variable declaration and thinks "welp, it's just going to end up in
an address register anyway, may as well optimize it away" at even really
low optimization levels, so I'd be surprised if there's even a difference
in the machine code generated, though it might hinder inlining the function
as a whole. Mostly I'm just noticing the inconsistent coding style.

Good points about the alias: I can see where the alias variable acts as a
reminder of the data type, but even that is the type of a pointer to the
struct, so we're still 1 level of indirection away instead of 2. I can also
see where it might just be an artifact of getting stuff to fit within 80
chars.

Bad points about the alias: I can see where things might hinder function
inlining, and might also hinder git grep searches for the global variable
name (though the global in question is scoped to the file, so that's not a
big concern).

Is this something worth standardizing, and if so, which style do we like
better?

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-08-20 03:25:00 Re: Nitpick/question: Use of aliases for global variables in functions
Previous Message Kyotaro Horiguchi 2021-08-20 02:53:08 Re: Some leftovers of recent message cleanup?