Обсуждение: Do we want a xmalloc or similar function in the Backend?

Hi,

There are 70+ calls of malloc in the backend in the form of

type* foo = malloc(sizeof(...));
if(!foo)  elog(ERROR, "could not allocate memory");

which is a bit annoying to write at times. Would somebody argue against 
introducing a function that does the above named xmalloc() or malloc_or_die()?

Greetings,

Andres
-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

Andres Freund <andres@2ndquadrant.com> writes:
> There are 70+ calls of malloc in the backend in the form of

> type* foo = malloc(sizeof(...));
> if(!foo)
>    elog(ERROR, "could not allocate memory");

> which is a bit annoying to write at times. Would somebody argue against 
> introducing a function that does the above named xmalloc() or malloc_or_die()?

99% of the time, you should be using palloc if that's the behavior you
want.  I think most of the malloc calls are in places where we want a
bit more control over the error response.
        regards, tom lane

On Tuesday, June 19, 2012 04:38:56 PM Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > There are 70+ calls of malloc in the backend in the form of
> > 
> > type* foo = malloc(sizeof(...));
> > if(!foo)
> > 
> >    elog(ERROR, "could not allocate memory");
> > 
> > which is a bit annoying to write at times. Would somebody argue against
> > introducing a function that does the above named xmalloc() or
> > malloc_or_die()?
> 
> 99% of the time, you should be using palloc if that's the behavior you
> want.  I think most of the malloc calls are in places where we want a
> bit more control over the error response.
There are surprisingly many calls that just have the above logic without a 
more elaborate error message. Its mostly cases which allocate memory just once 
and never release it again to avoid having huge static data around for 
processes not using that part of the code.
True enough, most of those could use TopMemoryContext, its a rather 
established pattern not to do so in those cases though...

Andres
-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

On Tue, Jun 19, 2012 at 10:17 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> There are 70+ calls of malloc in the backend in the form of
>
> type* foo = malloc(sizeof(...));
> if(!foo)
>   elog(ERROR, "could not allocate memory");
>
> which is a bit annoying to write at times. Would somebody argue against
> introducing a function that does the above named xmalloc() or malloc_or_die()?

I can't even find 70 malloc calls in the entire backend, let alone 70
with that pattern.  Still, I don't think malloc_or_error (not die)
would be a bad idea.

But the error should definitely be written as:
                       ereport(ERROR,                                       (errcode(ERRCODE_OUT_OF_MEMORY),
                           errmsg("out of memory"))); 

...not elog.

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

On Tuesday, June 19, 2012 07:35:53 PM Robert Haas wrote:
> On Tue, Jun 19, 2012 at 10:17 AM, Andres Freund <andres@2ndquadrant.com> 
wrote:
> > There are 70+ calls of malloc in the backend in the form of
> > 
> > type* foo = malloc(sizeof(...));
> > if(!foo)
> >   elog(ERROR, "could not allocate memory");
> > 
> > which is a bit annoying to write at times. Would somebody argue against
> > introducing a function that does the above named xmalloc() or
> > malloc_or_die()?
> 
> I can't even find 70 malloc calls in the entire backend, let alone 70
> with that pattern.  Still, I don't think malloc_or_error (not die)
> would be a bad idea.
$ ack '\bmalloc\s*\(' src/backend/|wc -l
70

10-15 or so of those are comments.

The 70+ came from me running on some development branch with commitfest 
patches applied...

> But the error should definitely be written as:
> 
>                         ereport(ERROR,
>                                         (errcode(ERRCODE_OUT_OF_MEMORY),
>                                          errmsg("out of memory")));
> 
> ...not elog.
Yes, definitely. Currently some of those locations (e.g. in xlog.c) are only 
protected by Asserts... 

Andres
-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services