From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Tiny patch: sigmask.diff |
Date: | 2016-04-04 14:48:56 |
Message-ID: | 30419.1459781336@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2016-04-04 14:59:41 | Re: Timeline following for logical slots |
Previous Message | Tom Lane | 2016-04-04 14:24:33 | Re: postgres_fdw : altering foreign table not invalidating prepare statement execution plan. |