Re: Fix bogus use of "long" in aset.c

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix bogus use of "long" in aset.c
Date: 2025-10-30 00:19:58
Message-ID: CAApHDvrMWfHO4QtQLsf2HnH5i3=sKxDs2x4hiZaXWqvZHwZ_Og@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 30 Oct 2025 at 12:55, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > I did also consider [u]intptr_t, but thought Size was better as that's
> > what chsize is.
>
> Seems like it's important that the value be signed, so maybe ssize_t?
> Or ptrdiff_t?

I did think about this a bit. I did think that signed would make sense
for a few reasons, e.g if somehow the block's freeptr was a lower
address than the block. Since this is code that's checking for the
sanity of the context, maybe we shouldn't assume that's true (although
it should always be true).

The reason I left it an unsigned type was that the check is doing: if
(chsize + ALLOC_CHUNKHDRSZ != blk_used), so we'd still catch this with
an unsigned type, even if it wrapped due to going negative due to a
bogus freeptr. Changing to a signed type would leave us with a few
tests comparing signed to unsigned types. I wanted to try and avoid
that. I thought about making chsize signed, but that is also used for
storing the return value of GetChunkSizeFromFreeListIdx(), which is
unsigned. I just don't quite see a combination of changing the types
that doesn't leave us comparing signed with unsigned types. In any
case, nothing is checking for pointers being less or greater than
other pointers, so it's not like we'll miss catching something due to
not using signed types.

David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-10-30 00:22:13 Re: Use BumpContext contexts for TupleHashTables' tablecxt
Previous Message Jeff Davis 2025-10-30 00:08:47 Re: Use BumpContext contexts for TupleHashTables' tablecxt