Tiny patch: sigmask.diff

Aleksander Alekseev <a.alekseev@postgrespro.ru> 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

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/