Re: Recursive ReceiveSharedInvalidMessages not safe - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Recursive ReceiveSharedInvalidMessages not safe
Date
Msg-id 24366.1399330259@sss.pgh.pa.us
Whole thread Raw
In response to Re: Recursive ReceiveSharedInvalidMessages not safe  (Andres Freund <andres@2ndquadrant.com>)
Responses Re: Recursive ReceiveSharedInvalidMessages not safe  (Andres Freund <andres@2ndquadrant.com>)
List pgsql-hackers
Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-05-05 15:41:22 -0400, Tom Lane wrote:
>> Looks all right to me.  Yeah, the right shift might have undefined
>> high-order bits, but we don't care because we're storing the result
>> into an int16.

> Doesn't at the very least
>         rnode.backend = (msg->sm.backend_hi << 16) | (int) msg->sm.backend_lo;
> have to be
>         rnode.backend = ((int) msg->sm.backend_hi << 16) | (int) msg->sm.backend_lo;

A promotion to int (or wider) is implicit in any arithmetic operation,
last I checked the C standard.  The "(int)" on the other side isn't
necessary either.

We might actually be better off casting both sides to unsigned int,
just to enforce that the left shifting is done with unsigned semantics.

>>> c) The ProcessMessageList() calls access msg->rc.id to test for the type
>>> of the existing message. That's not nice.

>> Huh?

> The checks should access msg->id, not msg->rc.id...

Meh.  If they're not the same thing, we've got big trouble anyway.
But if you want to change it as a cosmetic thing, no objection.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Jeff Janes
Date:
Subject: 9.4 wrap around issues
Next
From: Tom Lane
Date:
Subject: Re: pg_shmem_allocations view