public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nathan Bossart <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: [email protected]
Subject: Re: Unneeded volatile qualifier in fmgr.c
Date: Wed, 12 Feb 2025 09:38:04 -0600
Message-ID: <Z6zAXAKSqQHOwqyE@nathan> (raw)
In-Reply-To: <[email protected]>
References: <Z6xbAgXKY2L-3d5Q@jrouhaud>
	<[email protected]>

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






view thread (3+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Unneeded volatile qualifier in fmgr.c
  In-Reply-To: <Z6zAXAKSqQHOwqyE@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox