On 2016-10-04 21:40:29 -0400, Tom Lane wrote:
> Andres Freund <and...@anarazel.de> writes:
> > On 2016-10-05 09:38:15 +0900, Michael Paquier wrote:
> >> The existing interface of MemoryContextAlloc do not care much about
> >> anything except Size, so I'd just give the responsability to the
> >> caller to do checks like queue != (Size) queue when queue is a uint64
> >> for example.
> > Well, that duplicates the check and error message everywhere.
> It seems like you're on the edge of reinventing calloc(), which is not an
> API that anybody especially likes.

I'm not sure how doing an s/Size/uint64/ in a bunch of APIs does
that. Because that'd allow us to to throw an error in a number of useful
cases where we currently can't.

I'm mostly concerned that there's a bunch of cases on 32bit platforms
where size_t is trivially overflowed. And being a bit more defensive
against that seems like a good idea. It took about a minute (10s of that
due to a typo) to find something that looks borked to me:
spi_printtup(TupleTableSlot *slot, DestReceiver *self)
        if (tuptable->free == 0)
                /* Double the size of the pointer array */
                tuptable->free = tuptable->alloced;
                tuptable->alloced += tuptable->free;
                tuptable->vals = (HeapTuple *) repalloc_huge(tuptable->vals,
tuptable->alloced * sizeof(HeapTuple));
seems like it could overflow quite easily on a 32bit system.

People don't think about 32bit size_t a whole lot anymore, so I think by
defaulting to 64bit calculations for this kind of thing, we'll probably
prevent a number of future bugs.


Andres Freund

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to