Hi,
On 2016-06-02 19:15:07 +0100, Greg Stark wrote:
> Ok. I added a comment and also fixed a small typo.
> diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
> index 3fbe0e5..a96fb7b 100644
> --- a/src/backend/storage/ipc/latch.c
> +++ b/src/backend/storage/ipc/latch.c
> @@ -485,35 +485,40 @@ CreateWaitEventSet(MemoryContext context, int nevents)
> char *data;
> Size sz = 0;
>
> - sz += sizeof(WaitEventSet);
> - sz += sizeof(WaitEvent) * nevents;
> + /*
> + * struct epoll_event contains a uint64_t which on some architectures
> + * requires 8-byte alignment and just to be safe be prepared for future
> + * additions of other such elements later in the WaitEventSet structure
> + */
I'd rather phrase this roughly like:
Use MAXALIGN size/alignment to guarantee that later uses of memory are
aligned correctly. E.g. epoll_event might need 8 byte alignment on some
platforms, but earlier allocations like WaitEventSet and WaitEvent might
not sized to guarantee that when purely using sizeof().
> + sz += MAXALIGN(sizeof(WaitEventSet));
> + sz += MAXALIGN(sizeof(WaitEvent) * nevents);
>
> #if defined(WAIT_USE_EPOLL)
> - sz += sizeof(struct epoll_event) * nevents;
> + sz += MAXALIGN(sizeof(struct epoll_event) * nevents);
> #elif defined(WAIT_USE_POLL)
> - sz += sizeof(struct pollfd) * nevents;
> + sz += MAXALIGN(sizeof(struct pollfd) * nevents);
> #elif defined(WAIT_USE_WIN32)
> /* need space for the pgwin32_signal_event */
> - sz += sizeof(HANDLE) * (nevents + 1);
> + sz += MAXALIGN(sizeof(HANDLE) * (nevents + 1));
> #endif
>
> data = (char *) MemoryContextAllocZero(context, sz);
>
> set = (WaitEventSet *) data;
> - data += sizeof(WaitEventSet);
> + data += MAXALIGN(sizeof(WaitEventSet));
>
> set->events = (WaitEvent *) data;
> - data += sizeof(WaitEvent) * nevents;
> + data += MAXALIGN(sizeof(WaitEvent) * nevents);
>
> #if defined(WAIT_USE_EPOLL)
> set->epoll_ret_events = (struct epoll_event *) data;
> - data += sizeof(struct epoll_event) * nevents;
> + data += MAXALIGN(sizeof(struct epoll_event) * nevents);
> #elif defined(WAIT_USE_POLL)
> set->pollfds = (struct pollfd *) data;
> - data += sizeof(struct pollfd) * nevents;
> + data += MAXALIGN(sizeof(struct pollfd) * nevents);
> #elif defined(WAIT_USE_WIN32)
> set->handles = (HANDLE) data;
> - data += sizeof(HANDLE) * nevents;
> + data += MAXALIGN(sizeof(HANDLE) * nevents);
> #endif
You can't easily test WIN32, but I'd suggest running make check with
WAIT_USE_POLL and WAIT_USE_SELECT at the top.
Looks sane otherwise.
Greetings,
Andres Freund