On Wed, Oct 5, 2016 at 2:09 PM, Andres Freund <and...@anarazel.de> wrote:
> 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:
> 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.

I think you're right, but I also think that if we start using uint64
rather than Size for byte-lengths, it will spread like kudzu through
the whole system and we'll lose the documentation benefit of having
sizes be called "Size".  Since descriptive type names are a good
thing, I don't like that very much.  One crazy idea is to change Size
to always be 64 bits and fix all the places where we translate between
Size and size_t.  But I'm not sure that's a good idea, either.  This
might be one of those cases where it's best to just accept that we're
going to miss some things and fix them as we find them.

Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

Reply via email to