From: | Jacob Brazeal <jacob(dot)brazeal(at)gmail(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Confusing variable naming in LWLockRelease |
Date: | 2025-01-30 07:16:34 |
Message-ID: | CA+COZaCov2kUf4WDpa+M1jbLpgh6SYxjBFykYnUndDu0cJYaqA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello all,
In the LWLockRelease function, we have the following snippet:
*/**
* * Release my hold on lock, after that it can immediately be acquired by *
others, even if we still have to wakeup other waiters. */ if (mode ==
LW_EXCLUSIVE) oldstate = pg_atomic_sub_fetch_u32(&lock->state,
LW_VAL_EXCLUSIVE); else oldstate = pg_atomic_sub_fetch_u32(&lock->state,
LW_VAL_SHARED);*
Here the variable name "oldstate" leads one to believe that the value is
fetched before the sub operation, similar to some other usages in lwlock.c.
However, as the documentation for `pg_atomic_sub_fetch_u32 states, "Returns
the value of ptr after the arithmetic operation", which is, of course, the
intended and correct behavior in this context.
Would it make sense to rename this field to, say, *newstate*? I think it'd
help the readability for the function, as the logic doesn't make sense if
the value is fetched before the write. I attached a patch in case we'd want
to do this.
Attachment | Content-Type | Size |
---|---|---|
v1-oldstate.patch | application/octet-stream | 1.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Vladyslav Nebozhyn | 2025-01-30 07:23:40 | Re: Feature Request: Add AES-128-CFB Mode Support to pgcrypto |
Previous Message | Peter Smith | 2025-01-30 07:12:22 | Re: Skip collecting decoded changes of already-aborted transactions |