On Mon, Oct 7, 2013 at 4:47 PM, Andres Freund <and...@2ndquadrant.com> wrote: > On 2013-10-07 13:25:17 -0400, Robert Haas wrote: >> On Fri, Oct 4, 2013 at 8:19 AM, Andres Freund <and...@2ndquadrant.com> wrote: >> > Could it be that MAXALIGN/TYPEALIGN doesn't really work for values >> > bigger than 32bit? >> > >> > #define MAXALIGN(LEN) TYPEALIGN(MAXIMUM_ALIGNOF, (LEN)) >> > #define TYPEALIGN(ALIGNVAL,LEN) \ >> > (((intptr_t) (LEN) + ((ALIGNVAL) - 1)) & ~((intptr_t) ((ALIGNVAL) >> > - 1))) >> >> Isn't the problem, more specifically, that it doesn't work for values >> larger than an intptr_t? > > Well, yes. And intptr_t is 32bit wide on a 32bit platform. > >> And does that indicate that intptr_t is the wrong type to be using here? > > No, I don't think so. intptr_t is defined to be a integer type to which > you can cast a pointer, cast it back and still get the old value. On > 32bit platforms it usually will be 32bit wide. > All that's fine for the classic usages of TYPEALIGN where it's used on > pointers or lengths of stuff stored in memory. Those will always fit in > 32bit on a 32bit platform. But here we're using it on explicit 64bit > types (XLogRecPtr). > Now, you could argue that we should make it use 64bit math everywhere - > but I think that might incur quite the price on some 32bit > platforms. It's used in the tuple decoding stuff, that's quite the hot > path in some workloads. > > So I guess it's either a separate macro, or we rewrite that piece of > code to work slightly differently and work directly on the lenght or > such. > > Maybe we should add a StaticAssert ensuring the TYPEALIGN macro only > gets passed 32bit types?
I think having two macros that behave identically on all platforms anyone cares about[1] but which can cause difficult-to-find corner-case-bugs on other platforms is just a recipe for disaster. I pledge to screw that up at least once. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company [1] And by anyone, I mean me. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers