Unneeded volatile qualifier in fmgr.c
Hi,
While reading some code in fmgr.c I noticed that the save_nestlevel variable is
declared as volatile. I'm assuming that's because a long time ago it was
modified in the PG_TRY / PG_CATCH block but it doesn't look needed anymore.
Trivial patch attached.
Attachments:
fmgr_volatile.difftext/plain; charset=us-asciiDownload
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index aa89ae8fe1a..ef0fde51b38 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -639,7 +639,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
ListCell *lc1,
*lc2,
*lc3;
- volatile int save_nestlevel;
+ int save_nestlevel;
PgStat_FunctionCallUsage fcusage;
if (!fcinfo->flinfo->fn_extra)
Hi,
On Wed, Feb 12, 2025 at 04:25:38PM +0800, Julien Rouhaud wrote:
Hi,
While reading some code in fmgr.c I noticed that the save_nestlevel variable is
declared as volatile. I'm assuming that's because a long time ago it was
modified in the PG_TRY / PG_CATCH block but it doesn't look needed anymore.Trivial patch attached.
Yeah, makes sense to me.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Wed, Feb 12, 2025 at 08:41:34AM +0000, Bertrand Drouvot wrote:
On Wed, Feb 12, 2025 at 04:25:38PM +0800, Julien Rouhaud wrote:
While reading some code in fmgr.c I noticed that the save_nestlevel variable is
declared as volatile. I'm assuming that's because a long time ago it was
modified in the PG_TRY / PG_CATCH block but it doesn't look needed anymore.Trivial patch attached.
Yeah, makes sense to me.
It was modified in the PG_CATCH section when it was first added in commit
2abae34, but that part was removed a week later in commit 82a4798. But I'm
not even sure it needed the volatile marker in the first commit, based on
the comment in elog.h:
* Note: if a local variable of the function containing PG_TRY is modified
* in the PG_TRY section and used in the PG_CATCH section, that variable
* must be declared "volatile" for POSIX compliance.
AFAICT it was never modified in the PG_TRY section.
In any case, I don't see a problem with removing "volatile" now, so I'll
plan on committing this later today if there are no objections.
--
nathan
On Wed, Feb 12, 2025 at 09:38:04AM -0600, Nathan Bossart wrote:
In any case, I don't see a problem with removing "volatile" now, so I'll
plan on committing this later today if there are no objections.
Committed.
--
nathan
On Wed, Feb 12, 2025 at 03:47:14PM -0600, Nathan Bossart wrote:
On Wed, Feb 12, 2025 at 09:38:04AM -0600, Nathan Bossart wrote:
In any case, I don't see a problem with removing "volatile" now, so I'll
plan on committing this later today if there are no objections.Committed.
Thanks.