Re: [HACKERS] Tiny patch: sigmask.diff

[Aleksander Alekseev]

> 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

[Tom Lane]

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