Re: Portability issues in shm_mq

On Sun, Mar 16, 2014 at 10:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> But I think there's another possible problem here.  In order for reads
>> from the buffer not to suffer alignment problems, the chunk size for
>> reads and writes from the buffer needs to be MAXIMUM_ALIGNOF (or some
>> multiple of it).  And in order to avoid a great deal of additional and
>> unwarranted complexity, the size of the message word also needs to be
>> MAXIMUM_ALIGNOF (or some multiple of it).  So the message word can
>> only be of size 4 if MAXIMUM_ALIGNOF is also 4.  IOW, I think your
>> approach is going to run into trouble on any system where
>> sizeof(Size)==4 but MAXIMUM_ALIGNOF==8.
>
> Well, it will result in padding space when you maxalign the length word,
> but I don't see why it wouldn't work; and it would certainly be no less
> efficient than what's there today.

Well, the problem is with this:
   /* Write the message length into the buffer. */   if (!mqh->mqh_did_length_word)   {       res =
shm_mq_send_bytes(mqh,sizeof(uint64), &nbytes, nowait,                               &bytes_written);
 

If I change nbytes to be of type Size, and the second argument to
sizeof(Size), then it's wrong whenever sizeof(Size) % MAXIMUM_ALIGNOF
!= 0.  I could do something like:

union
{   char pad[MAXIMUM_ALIGNOF];   Size val;
} padded_size;

padded_size.val = nbytes;
res = shm_mq_send_bytes(mqh, sizeof(padded_size), &padded_size,
nowait, &bytes_written);

...but that probably *is* less efficient, and it's certainly a lot
uglier; a similar hack will be needed when extracting the length word,
and assertions elsewhere will need adjustment.  I wonder if it
wouldn't be better to adjust the external API to use Size just for
consistency but, internally to the module, keep using 8 byte sizes
within the buffer.  Really, I think that would boil down to going
through and making sure that we use TYPEALIGN(...,sizeof(uint64))
everywhere instead of MAXALIGN(), which doesn't seem like a big deal.
On the third hand, maybe there are or will be platforms out there
where MAXIMUM_ALIGNOF > 8.  If so, it's probably best to bite the
bullet and allow for padding now, so we don't have to monkey with this
again later.

Sorry for belaboring this point, but I want to make sure I only need
to fix this once.

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