Re: unchecked malloc

Started by Tom Laneover 20 years ago4 messages
#1Tom Lane
tgl@sss.pgh.pa.us

"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:

There are several places in both backend and tools that forget to check the
return value of malloc(). For example(8.0.1),

backend/port/dynloader/beos.c/pg_dlopen()

Dead port, probably not worth fixing.

backend/bootstrap/bootstrap.c/AddStr()

This code will never be run under memory pressure, so although
it theoretically should be fixed, I'm having a hard time getting
excited about it. In any case palloc would be the correct fix.

port/strdup.c/strdup()

Definitely broken; I just fixed it to conform to the SUS
http://www.opengroup.org/onlinepubs/007908799/xsh/strdup.html
(of course, this code isn't used on any remotely modern
platform, so it's probably not very relevant...)

bin/pg_dump/common.c/findParentsByOid()

Probably should be fixed, although if pg_dump runs out of memory
it's just gonna fail anyway.

To report the "out of memory" error, we should differenciate if
ErrorContext is already set up.

No, because you're thinking in terms of the backend environment, and
generally in the backend the answer to "when to use malloc directly"
is "never". The client-side tools are the only place where this is
a serious question, and offhand I'd say "gripe to stderr and exit(1)"
is going to be the right answer in all cases there.

regards, tom lane

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#1)

"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:

"Tom Lane" <tgl@sss.pgh.pa.us> wrote

No, because you're thinking in terms of the backend environment, and
generally in the backend the answer to "when to use malloc directly"
is "never".

Well, except before MemoryContext mechanism is set up? For example, the
functions(e.g., GUC, vfd) used during bootstrap.

I think you need to take another look at the startup sequences. Those
modules are not run before MemoryContextInit. In any case, the odds
of running out of memory before we get to MemoryContextInit are so small
that I don't have a problem with crashing if it happens.

regards, tom lane

#3Sibtay Abbas
sibtay@gmail.com
In reply to: Tom Lane (#2)

This dicussion reminds me of a possible memory leak in plpgsql's code. In
case you are interested in it;

in pl_comp.c, plpgsql_build_variable takes a pointer to a PLpgSQL_type
structure, which is always a malloc'ed instance(since we always use
plpgsql_build_datatype function). The switch statement in
plpgsql_build_variable function elicits that its reference is only kept in
case the type structure represents a PLPGSQL_TTYPE_SCALAR, otherwise it is
not kept and needed in case its either PLPGSQL_TTYPE_ROW or
PLPGSQL_TTYPE_REC.

So is it intensional or a memory leak?

Thank you

Show quoted text

On 9/27/05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:

"Tom Lane" <tgl@sss.pgh.pa.us> wrote

No, because you're thinking in terms of the backend environment, and
generally in the backend the answer to "when to use malloc directly"
is "never".

Well, except before MemoryContext mechanism is set up? For example, the
functions(e.g., GUC, vfd) used during bootstrap.

I think you need to take another look at the startup sequences. Those
modules are not run before MemoryContextInit. In any case, the odds
of running out of memory before we get to MemoryContextInit are so small
that I don't have a problem with crashing if it happens.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Sibtay Abbas (#3)

Sibtay Abbas <sibtay@gmail.com> writes:

in pl_comp.c, plpgsql_build_variable takes a pointer to a PLpgSQL_type
structure, which is always a malloc'ed instance(since we always use
plpgsql_build_datatype function).

As of current sources it's palloc'd, and should be released if the
function is ever recompiled, so I see no strong reason to worry.

regards, tom lane