Re: C nitpick about pgwin32_dispatch_queued_signals()
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Bryan Green <dbryan.green@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2025-11-02T16:47:45Z
Lists: pgsql-hackers
Bryan Green <dbryan.green@gmail.com> writes: > On 11/2/2025 7:05 AM, Christian Ullrich wrote: >> 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). > 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. Agreed. > 2. Restructure to use && instead of ?:: > #define INTERRUPTS_PENDING_CONDITION() \ > (unlikely(UNBLOCKED_SIGNAL_QUEUE()) && \ > (pgwin32_dispatch_queued_signals(), true), \ > unlikely(InterruptPending)) > I'd lean towards #2 --- it's cleaner and avoids the whole issue. I dunno, don't really like the nested comma operator here. It's probably necessary to avoid having a void argument to &&, but it's confusing. Let's go with your #1 (casting the 0 to void). But can't we simplify that to just #define INTERRUPTS_PENDING_CONDITION() \ (unlikely(UNBLOCKED_SIGNAL_QUEUE()) ? pgwin32_dispatch_queued_signals() : (void) 0, \ unlikely(InterruptPending)) #endif that is, the only change needed is s/0/(void) 0/. regards, tom lane
Commits
-
Avoid mixing void and integer in a conditional expression.
- 78d542b2bc0a 13.23 landed
- d26997c71281 14.20 landed
- cd55abab4fa4 15.15 landed
- d5e9c7115027 16.11 landed
- 436d9ed690eb 17.7 landed
- fdbc67572762 18.1 landed
- 645c1e2752d9 19 (unreleased) landed