Looking through the postgresql source code, I notice that there are many places
were palloc is used but the return value is not checked to see if it is null.
There are a few places such as:

if (!PointerIsValid(result = palloc(CASH_BUFSZ + 2 - count +
strlen(nsymbol))))
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));

(taken from src/backend/utils/adt/cash.c), but at least within the that same
directory most occurrences of palloc are not checked.

Is this sloppy programming, or is there an automagical thing going on with
#defines that I'm just not seeing?

If it is just sloppy, perhaps we could use a new define in palloc.h, such as:

#define palloc_or_die(ptr,sz) \
do { \
ptr = palloc(sz); \
if (!ptr) \
{ \
ereport(ERROR, \
(errcode(ERRCODE_OUT_OF_MEMORY), \
errmsg("Out of memory"))); \
} \
} while(0);

And then, in all places where the code does not currently check the return value
of palloc, the code could be changed to use palloc_or_die instead. Of course,
I'd be happy if someone has a better name for the macro, perhaps something more
brief?

I can go over the code "with a fine tooth comb" and replace the offending
occurrences. Does the community think this is a good idea?

mark

Mark Dilger <pgsql@markdilger.com> writes:
> Looking through the postgresql source code, I notice that there are many places
> were palloc is used but the return value is not checked to see if it is null.
Any place that *does* check it is incorrect (in the sense of being
wasted code space, not functionally incorrect). Please read the memmgr
documentation.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Mark Dilger wrote:
> Looking through the postgresql source code, I notice that there are
> many places were palloc is used but the return value is not checked
> to see if it is null.

palloc will throw an exception if it cannot fulfill the request. Code
that checks the return value for null is in fact a waste.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Peter Eisentraut wrote:
> Mark Dilger wrote:
>
>>Looking through the postgresql source code, I notice that there are
>>many places were palloc is used but the return value is not checked
>>to see if it is null.
>
>
> palloc will throw an exception if it cannot fulfill the request. Code
> that checks the return value for null is in fact a waste.
>

Interesting. So the patch should go the other way, and remove the checks that
are currently in the code?

Mark Dilger wrote:
> Peter Eisentraut wrote:
> >Mark Dilger wrote:
> >
> >>Looking through the postgresql source code, I notice that there are
> >>many places were palloc is used but the return value is not checked
> >>to see if it is null.
> >
> >palloc will throw an exception if it cannot fulfill the request. Code
> >that checks the return value for null is in fact a waste.
>
> Interesting. So the patch should go the other way, and remove the checks
> that are currently in the code?

Yes, if you find any such place please submit a patch.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Alvaro Herrera wrote:
> Mark Dilger wrote:
> > Peter Eisentraut wrote:
> > >Mark Dilger wrote:
> > >
> > >>Looking through the postgresql source code, I notice that there are
> > >>many places were palloc is used but the return value is not checked
> > >>to see if it is null.
> > >
> > >palloc will throw an exception if it cannot fulfill the request. Code
> > >that checks the return value for null is in fact a waste.
> >
> > Interesting. So the patch should go the other way, and remove the checks
> > that are currently in the code?
>
> Yes, if you find any such place please submit a patch.

I see one in cash.c that I will remove.

--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com

+ If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

On Sun, 2006-03-19 at 17:14 -0500, Bruce Momjian wrote:
> I see one in cash.c that I will remove.

I've already checked in a fix for that, as well as a few other places
that made similar mistakes -- sorry for stepping on your toes.

-Neil



---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend