pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Bryan Green <dbryan.green@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Subject: Re: C nitpick about pgwin32_dispatch_queued_signals()
Date: Sun, 2 Nov 2025 09:29:43 -0600
Message-ID: <9c55d148-17c5-4d3a-96a1-7ece939a202a@gmail.com> (raw)
In-Reply-To: <cc4ef8db-f8dc-4347-8a22-e7ebf44c0308@chrullrich.net>
References: <cc4ef8db-f8dc-4347-8a22-e7ebf44c0308@chrullrich.net>

On 11/2/2025 7:05 AM, Christian Ullrich wrote:
> Hello,
> 
> the current MSVC compiler deems it necessary to issue
> 
> 	warning C4053: one void operand for '?:'
> 
> for a line with CHECK_FOR_INTERRUPTS(). This boils down to this bit of
> miscadmin.h (line 116 in master):
> 
> 	#define INTERRUPTS_PENDING_CONDITION() \
> 		(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
> 			pgwin32_dispatch_queued_signals() : 0, \
> 		 unlikely(InterruptPending))
> 	#endif
> 
> The C spec says that of the possible results of the :? operator, either
> none or both can be void, and pgwin32_dispatch_queued_signals() is void
> (and has been as far back as I can find it).
> 
> Does that matter?
> 
> 
Yeah, this is a bug, or at least a spec violation.  We should fix it in 
my opinion-- it's non-conforming C. Others may disagree, though.

It happens to work because the comma operator discards the result 
anyway, but MSVC is right to complain.

This isn't a new thing with modern C standards, BTW --- the rule
about not mixing void and non-void operands in ?: has been there
since C89.  We've just been getting away with it because most
compilers don't complain when the result is discarded by the
comma operator anyway.

I see a couple of ways to fix it:

1. Cast both arms to void:

     #define INTERRUPTS_PENDING_CONDITION() \
         (unlikely(UNBLOCKED_SIGNAL_QUEUE()) ? \
             (pgwin32_dispatch_queued_signals(), (void)0) : (void)0, \
          unlikely(InterruptPending))

2. Restructure to use && instead of ?::

     #define INTERRUPTS_PENDING_CONDITION() \
         (unlikely(UNBLOCKED_SIGNAL_QUEUE()) && \
             (pgwin32_dispatch_queued_signals(), true), \
          unlikely(InterruptPending))

3. Just make it an inline function instead of a macro.

I'd lean towards #2 --- it's cleaner and avoids the whole issue.
The semantics are the same since we're only calling the function
for its side-effects.

Thoughts?

Bryan Green





view thread (5+ messages)  latest in thread

Message-ID: <9c55d148-17c5-4d3a-96a1-7ece939a202a@gmail.com>
Permalink:  ../9c55d148-17c5-4d3a-96a1-7ece939a202a@gmail.com/
Also on:    postgresql.org/message-id/9c55d148-17c5-4d3a-96a1-7ece939a202a@gmail.com

 · 

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: pgsql-hackers@postgresql.org
  Cc: dbryan.green@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: C nitpick about pgwin32_dispatch_queued_signals()
  In-Reply-To: <9c55d148-17c5-4d3a-96a1-7ece939a202a@gmail.com>

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

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