From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Antonin Houska <ah(at)cybertec(dot)at> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Thinko in processing of SHM message size info? |
Date: | 2015-08-06 17:29:20 |
Message-ID: | CA+TgmoZs0mtbLGCF5xp+Rc4Na+NEyvt+Ubqk_9+uUY__FhsN+Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Aug 6, 2015 at 1:24 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Aug 6, 2015 at 9:04 AM, Antonin Houska <ah(at)cybertec(dot)at> wrote:
>> Can anyone please explain why the following patch shouldn't be applied?
>>
>> diff --git a/src/backend/storage/ipc/shm_mq.c b/src/backend/storage/ipc/shm_mq.c
>> index 126cb07..4cd52ac 100644
>> --- a/src/backend/storage/ipc/shm_mq.c
>> +++ b/src/backend/storage/ipc/shm_mq.c
>> @@ -584,7 +584,7 @@ shm_mq_receive(shm_mq_handle *mqh, Size *nbytesp, void **datap, bool nowait)
>> if (mqh->mqh_partial_bytes + rb > sizeof(Size))
>> lengthbytes = sizeof(Size) - mqh->mqh_partial_bytes;
>> else
>> - lengthbytes = rb - mqh->mqh_partial_bytes;
>> + lengthbytes = rb;
>> memcpy(&mqh->mqh_buffer[mqh->mqh_partial_bytes], rawdata,
>> lengthbytes);
>> mqh->mqh_partial_bytes += lengthbytes;
>>
>>
>> I'm failing to understand why anything should be subtracted. Note that the
>> previous iteration must have called shm_mq_inc_bytes_read(), so "rb" should
>> not include anything of mqh->mqh_partial_bytes. Thanks.
>
> Hmm, I think you are correct. This would matter in the case where the
> message length word was read in more than two chunks. I don't *think*
> that's possible right now because I believe the only systems where
> MAXIMUM_ALIGNOF < sizeof(Size) are those with MAXIMUM_ALIGNOF == 4 and
> sizeof(Size) == 8. However, if we had systems where MAXIMUM_ALIGNOF
> == 4 and sizeof(Size) == 16, or systems where MAXIMUM_ALIGNOF == 2 and
> sizeof(Size) == 8, this would be a live bug.
Hmm, actually, maybe it is a live bug anyway, because the if statement
tests > rather than >=. If we've read 4 bytes and exactly 4 more
bytes are available, we'd set lengthbytes to 0 instead of 4. Oops.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2015-08-06 17:48:26 | Re: 9.5 release notes |
Previous Message | Robert Haas | 2015-08-06 17:24:35 | Re: Thinko in processing of SHM message size info? |