Re: Missing checks when malloc returns NULL...

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Missing checks when malloc returns NULL...
Date: 2016-08-31 15:14:33
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> Which means that processes have an escape window when initializing
> shared memory by cleaning up the index if an entry cannot be found and
> then cannot be created properly. I don't think that it is a good idea
> to change that by forcing ShmemAlloc to fail. So I would tend to just
> have the patch attached and add those missing NULL-checks on all the
> existing ShmemAlloc() calls.

> Opinions?

I still think it'd be better to fix that as attached, because it
represents a net reduction not net addition of code, and it provides
a defense against future repetitions of the same omission. If only
4 out of 11 existing calls were properly checked --- some of them
adjacent to calls with checks --- that should tell us that we *will*
have more instances of the same bug if we don't fix it centrally.

I also note that your patch missed checks for two ShmemAlloc calls in
InitShmemAllocation and ShmemInitStruct. Admittedly, since those are
the very first such calls, it's highly unlikely they'd fail; but I think
this exercise is not about dismissing failures as improbable. Almost
all of these failures are improbable, given that we precalculate the
shmem space requirement.

regards, tom lane

Attachment Content-Type Size
malloc-nulls-v7.patch text/x-diff 6.5 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-08-31 15:23:27 Re: pg_sequence catalog
Previous Message Ivan Kartyshov 2016-08-31 14:57:28 less expensive pg_buffercache on big shmem