Hi, On 2018-01-02 19:08:49 -0500, Tom Lane wrote: > Now, the existing definition of the struct seems safe on all > architectures we support, but it would not take much to break it. > I think we ought to do what we did recently in the memory-context > code: insert an explicit padding calculation and a static assertion > that it's right. Hence, the attached proposed patch (in which > I also failed to resist the temptation to make the two code paths > in dense_alloc() look more alike). Any objections?
Generally no objections. > + /* > + * It's required that data[] start at a maxaligned offset. This padding > + * calculation is correct as long as int is not wider than size_t and > + * dsa_pointer is not wider than a regular pointer, but there's a static > + * assertion to check things in nodeHash.c. > + */ But note that dsa_pointer can be wider than a regular pointer on platforms without atomics support. > +#define HASHMEMORYCHUNKDATA_RAWSIZE (SIZEOF_SIZE_T * 3 + SIZEOF_VOID_P) > + > + /* ensure proper alignment by adding padding if needed */ > +#if (HASHMEMORYCHUNKDATA_RAWSIZE % MAXIMUM_ALIGNOF) != 0 > + char padding[MAXIMUM_ALIGNOF - HASHMEMORYCHUNKDATA_RAWSIZE % > MAXIMUM_ALIGNOF]; > +#endif > + > char data[FLEXIBLE_ARRAY_MEMBER]; /* buffer allocated at > the end */ > } HashMemoryChunkData; Wonder if this specific case wouldn't be easier handled with a union? Greetings, Andres Freund
