Re: [HACKERS] Tiny patch: sigmask.diff
> Surely this fix is completely wrong? You'd have to touch every use of
> signum() to do it like that. You'd also be introducing similarly-
> undefined behavior at the other end of the loop, where this coding
> would be asking to compute 1<<31, hence shifting into the sign bit,
> which is undefined unless you make the computation explicitly
> unsigned.
Oh, I didn't think about that...
> I think better is just to change the for-loop to iterate from 1 not 0.
> Signal 0 is invalid anyway, and is rejected in pg_queue_signal for
> example, so there can't be any event waiting there.
Agreed.
--
Best regards,
Aleksander Alekseev
http://eax.me/
diff --git a/src/backend/port/win32/signal.c b/src/backend/port/win32/signal.c
index 36c6ebd..0ba2817 100644
--- a/src/backend/port/win32/signal.c
+++ b/src/backend/port/win32/signal.c
@@ -113,7 +113,7 @@ pgwin32_dispatch_queued_signals(void)
/* One or more unblocked signals queued for execution */
int exec_mask = UNBLOCKED_SIGNAL_QUEUE();
- for (i = 0; i < PG_SIGNAL_COUNT; i++)
+ for (i = 1; i < PG_SIGNAL_COUNT; i++)
{
if (exec_mask & sigmask(i))
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Tiny patch: sigmask.diff
Aleksander Alekseev writes: > sigmask macro is defined in win32.h like this: > #define sigmask(sig) ( 1 << ((sig)-1) ) > And used in signal.c in this fashion: > for (i = 0; i < PG_SIGNAL_COUNT; i++) > if (exec_mask & sigmask(i)) > Thus during first iteration we are doing `<< -1`. I think it's a bug. Agreed. > Patch attached. Surely this fix is completely wrong? You'd have to touch every use of signum() to do it like that. You'd also be introducing similarly- undefined behavior at the other end of the loop, where this coding would be asking to compute 1<<31, hence shifting into the sign bit, which is undefined unless you make the computation explicitly unsigned. I think better is just to change the for-loop to iterate from 1 not 0. Signal 0 is invalid anyway, and is rejected in pg_queue_signal for example, so there can't be any event waiting there. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Tiny patch: sigmask.diff
Hello
sigmask macro is defined in win32.h like this:
```
#define sigmask(sig) ( 1 << ((sig)-1) )
```
And used in signal.c in this fashion:
```
for (i = 0; i < PG_SIGNAL_COUNT; i++)
{
if (exec_mask & sigmask(i))
{
```
Thus during first iteration we are doing `<< -1`. I think it's a bug.
Patch attached.
--
Best regards,
Aleksander Alekseev
http://eax.me/
diff --git a/src/backend/port/win32/signal.c b/src/backend/port/win32/signal.c
index 36c6ebd..3724aa3 100644
--- a/src/backend/port/win32/signal.c
+++ b/src/backend/port/win32/signal.c
@@ -115,14 +115,14 @@ pgwin32_dispatch_queued_signals(void)
for (i = 0; i < PG_SIGNAL_COUNT; i++)
{
- if (exec_mask & sigmask(i))
+ if (exec_mask & sigmask(i+1))
{
/* Execute this signal */
pqsigfunc sig = pg_signal_array[i];
if (sig == SIG_DFL)
sig = pg_signal_defaults[i];
-pg_signal_queue &= ~sigmask(i);
+pg_signal_queue &= ~sigmask(i+1);
if (sig != SIG_ERR && sig != SIG_IGN && sig != SIG_DFL)
{
LeaveCriticalSection(&pg_signal_crit_sec);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
