On 2016-10-04 21:40:29 -0400, Tom Lane wrote:
> Andres Freund <andres@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:
bool
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.
Greetings,
Andres Freund
