Re: Tiny patch: sigmask.diff - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Tiny patch: sigmask.diff
Date
Msg-id 30419.1459781336@sss.pgh.pa.us
Whole thread Raw
In response to Tiny patch: sigmask.diff  (Aleksander Alekseev <a.alekseev@postgrespro.ru>)
Responses Re: Tiny patch: sigmask.diff  (Aleksander Alekseev <a.alekseev@postgrespro.ru>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: postgres_fdw : altering foreign table not invalidating prepare statement execution plan.
Next
From: Craig Ringer
Date:
Subject: Re: Timeline following for logical slots