Re: Missing checks when malloc returns NULL...

On 06/22/2016 04:41 AM, Michael Paquier wrote:
> On Tue, Jun 21, 2016 at 10:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>>> - mcxt.c uses that, which is surprising:
>>> @@ -704,7 +704,8 @@ MemoryContextCreate(NodeTag tag, Size size,
>>> {
>>> /* Special case for startup: use good ol' malloc */
>>> node = (MemoryContext) malloc(needed);
>>> - Assert(node != NULL);
>>> + if (node == NULL)
>>> + elog(PANIC, "out of memory");
>>> }
>>> I think that a PANIC is cleaner here instead of a simple crash.
>>
>> But the elog mechanism assumes that memory contexts are working.
>> If this ever actually did fire, no good would come of it.
> make s
> OK, there is not much that we can do here then. What about the rest?
> Those seem like legit concerns to me.

There's also a realloc() and an strdup() call in refint.c. But looking
at refint.c, I wonder why it's using malloc()/free() in the first place,
rather than e.g. TopMemoryContext. The string construction code with
sprintf() and strlen() looks quite antiqued, too, StringInfo would make
it more readable.

Does refint.c actually have any value anymore? What if we just removed
it altogether? It's good to have some C triggers in contrib, to serve as
examples, and to have some regression test coverage for all that. But
ISTM that the 'autoinc' module covers the trigger API just as well.

The code in ps_status() runs before the elog machinery has been
initialized, so you get a rather unhelpful error:

> error occurred at ps_status.c:167 before error message processing is available

I guess that's still better than outright crashing, though. There are
also a few strdup() calls there that can run out of memory..

Not many of the callers of simple_prompt() check for a NULL result, so
in all probability, returning NULL from there will just crash in the
caller. There's one existing "return NULL" in there already, so this
isn't a new problem, but can we find a better way?

There are more malloc(), realloc() and strdup() calls in isolationtester
and pg_regress, that we ought to change too while we're at it.

- Heikki