On 06/22/2016 04:41 AM, Michael Paquier wrote:
On Tue, Jun 21, 2016 at 10:46 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
Michael Paquier <michael.paqu...@gmail.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



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to