Re: Thinko in processing of SHM message size info? - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Thinko in processing of SHM message size info?
Date
Msg-id CA+TgmoarEChaXmnj6pRUoi=5TxUorpf5Jn7as_vr-eeBTeg1xQ@mail.gmail.com
Whole thread Raw
In response to Thinko in processing of SHM message size info?  (Antonin Houska <ah@cybertec.at>)
Responses Re: Thinko in processing of SHM message size info?
List pgsql-hackers
On Thu, Aug 6, 2015 at 9:04 AM, Antonin Houska <ah@cybertec.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.

Thanks for reviewing; I'll go push this change.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: "Joshua D. Drake"
Date:
Subject: Re: Removing unreferenced files
Next
From: Robert Haas
Date:
Subject: Re: Thinko in processing of SHM message size info?