signed vs. unsigned in TYPEALIGN (was Re: space reserved for WAL record does not match what was written: panic on windows)

On Thu, Oct 17, 2013 at 08:27:01PM +0200, Andres Freund wrote:
> On 2013-10-17 12:33:45 -0400, Noah Misch wrote:
> > > 1. Is there any guarantee that sizeof(intptr_t) >= sizeof(size_t)?
> > > (Note that Size is just a typedef for size_t, in c.h)
> >
> > C99 doesn't require it, but I have never heard of a platform where it is
> > false. sizeof(intptr_t) > sizeof(size_t) systems have existed.
>
> Either way, both have to be at least 4byte on 32bit platforms and 8byte
> on 64bit ones. So I as well think we're good.

C99 does not have concepts like "32bit platform" and "64bit platform", so it
cannot make such a constraint. Nonetheless, I agree we're good with respect
to implementations actually worth anticipating.

> > > 2. If intptr_t is a signed type, as it appears to be, and size_t is an
> > > unsigned type, as I believe it to be, then is it safe to use the
> > > macros written for the signed type with a value of the unsigned type?
> > > Off-hand I can't see a problem there, but I'm not certain I'm not
> > > missing something.
> >
> > Yes; we do that all the time, e.g. the MAXALIGN call in AllocSetAlloc().
> > Looping back to my question above, I think it doesn't matter (on a two's
> > complement system) whether the macro uses a signed type or an unsigned type.
> > It changes the type of the result; that's all. Nonetheless, we should be
> > consistent about signedness between the regular and 64-bit macro variants.
>
> I am not all that comfortable on relying on 2s complement here. Maybe we
> want to compile without -fwrapv one day which would make signed overflow
> undefined again. I don't think there are too many situations where the
> compiler would have the required knowledge to optimize stuff away,
> but...

Here is my position statement on that issue:
http://www.postgresql.org/message-id/20130220025819.GB6791@tornado.leadboat.com

> So I vote for only using unsigned arithmetic. I don't see anything
> preventing that?

TYPEALIGN has used signed math since the dawn of history, and TYPEALIGN64
departed from that precedent without comment. That led me to think of the
situation as prompting a fix for the oversight in TYPEALIGN64:

--- a/src/include/c.h
+++ b/src/include/c.h
@@ -560,3 +560,3 @@ typedef NameData *Name;
#define TYPEALIGN64(ALIGNVAL,LEN) \
- (((uint64) (LEN) + ((ALIGNVAL) - 1)) & ~((uint64) ((ALIGNVAL) - 1)))
+ (((int64) (LEN) + ((ALIGNVAL) - 1)) & ~((int64) ((ALIGNVAL) - 1)))

Having said that, changing the ancient macros to use uintptr_t does have the
advantage you mention, and I'm failing to think of a disadvantage.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com