Re: Recursive ReceiveSharedInvalidMessages not safe

Andres Freund <andres(at)2ndquadrant(dot)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