From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)2ndquadrant(dot)com> |
Cc: | David Rowley <dgrowleyml(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: space reserved for WAL record does not match what was written: panic on windows |
Date: | 2013-10-08 16:26:17 |
Message-ID: | CA+TgmoZ1CDDMY_vD0V3Ka+FW5CNMN0VsAySxLeWr2PQ77vKVxQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-www |
On Mon, Oct 7, 2013 at 4:47 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-10-07 13:25:17 -0400, Robert Haas wrote:
>> On Fri, Oct 4, 2013 at 8:19 AM, Andres Freund <andres(at)2ndquadrant(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2013-10-08 16:27:08 | Re: old warning in docs |
Previous Message | Bruce Momjian | 2013-10-08 16:20:55 | Re: unaccent module - two params function should be immutable |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2013-10-08 20:05:37 | Fwd: Undelivered Mail Returned to Sender |
Previous Message | Heikki Linnakangas | 2013-10-07 23:05:58 | Re: space reserved for WAL record does not match what was written: panic on windows |