Re: [COMMITTERS] pgsql: Add new function dsa_allocate0.

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Add new function dsa_allocate0.
Date: 2017-02-17 13:17:17
Message-ID: CA+Tgmobt7CcF_uQP2UQwWmu4K9qCHehMJP9_9m1urwP8hbOeHQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Thu, Feb 16, 2017 at 10:09 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Fri, Feb 17, 2017 at 12:03 PM, Thomas Munro
> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>> On Fri, Feb 17, 2017 at 11:34 AM, Thomas Munro
>> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>>> On Fri, Feb 17, 2017 at 7:02 AM, Robert Haas <rhaas(at)postgresql(dot)org> wrote:
>>>> http://git.postgresql.org/pg/commitdiff/9acb85597f1223ac26a5b19a9345849c43d0ff54
>>> Hmm. This will segfault if you're out of memory.
>>
>> Or to provide a more useful response... maybe this should be like the
>> attached? Or maybe people think that dsa_allocate should throw on
>> failure to allocate, like palloc?
>
> dp = dsa_allocate(area, size);
> - object = dsa_get_address(area, dp);
> - memset(object, 0, size);
> + if (DsaPointerIsValid(dp))
> + memset(dsa_get_address(area, dp), 0, size);
> What you are proposing here looks like the right answer to me. Like
> dsa_allocate, dsa_allocate0 should allow users to fallback to other
> methods if what is returned is InvalidDsaPointer for consistency.

I'm thinking we should change this to look more like the
MemoryContextAlloc interface. Let's have DSA_ALLOC_HUGE,
DSA_ALLOC_NO_OOM, and DSA_ALLOC_ZERO, just like the corresponding
MCXT_* flags, and a function dsa_allocate_extended() that takes a
flags argument. Then, dsa_allocate(x,y) can be a macro for
dsa_allocate_extended(x,y,0) and dsa_allocate0(x,y) can be a macro for
dsa_allocate_extended(x,y,DSA_ALLOC_ZERO). What this goof on my (and
Dilip's) part illustrates to me is that having this interface behave
significantly differently from the MemoryContextAlloc interface is
going to cause mistakes.

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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Eisentraut 2017-02-17 13:57:58 pgsql: Remove redundant coverage target
Previous Message Peter Eisentraut 2017-02-17 04:53:24 pgsql: Fix typo on comment

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-02-17 13:22:47 Re: Sum aggregate calculation for single precsion real
Previous Message Rushabh Lathia 2017-02-17 12:57:17 Re: Gather Merge