From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Andy Fan <zhihuifan1213(at)163(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | convert libpgport's pqsignal() to a void function |
Date: | 2025-01-15 02:45:12 |
Message-ID: | Z4chOKfnthRH71mw@nathan |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
(moving to a new thread)
On Mon, Jan 13, 2025 at 10:04:31AM -0600, Nathan Bossart wrote:
> On Sat, Jan 11, 2025 at 02:04:13PM -0500, Tom Lane wrote:
>> BTW, this decouples legacy-pqsignal.c from pqsignal.c enough
>> that we could now do what's contemplated in the comments from
>> 3b00fdba9: simplify that version by making it return void,
>> or perhaps better just a true/false success report.
>> I've not touched that point here, though.
>
> I think it should just return void since AFAICT nobody checks the return
> value. But it would probably be a good idea to add some sort of error
> checking within the function. I've attached a draft patch that adds some
> new assertions. I originally tried to use elog() where it was available,
> but besides making the code even more unreadable, I think it's unnecessary
> (since AFAICT any problems are likely coding errors).
With commit 9a45a89 in place, this is now possible. I've attached a new
version of the patch with a more fleshed-out commit message and a small
comment fix.
For background, the protections added by commit 3b00fdb introduced race
conditions to pqsignal() that could lead to bogus return values. That's of
little consequence since nobody seems to look at the return values, but it
would have been nice to convert it to a void function to avoid any
possibility of a bogus return value. Unfortunately, doing so would have
required also modifying legacy-pqsignal.c's version of the function, which
would've required an SONAME bump, which didn't seem worth it. Or so I
thought...
Thanks to commit 9a45a89, legacy-pqsignal.c now has its own dedicated
extern for pqsignal(), which decouples it enough that we can follow through
with changing libpqport's pqsignal() to a void function.
Thoughts?
--
nathan
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Convert-libpgport-s-pqsignal-to-a-void-function.patch | text/plain | 3.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2025-01-15 03:02:46 | Re: convert libpgport's pqsignal() to a void function |
Previous Message | Michail Nikolaev | 2025-01-15 00:39:37 | Re: Issue with markers in isolation tester? Or not? |