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
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 |
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 |