pgsql: Change API of ShmemAlloc() so it throws error rather than return

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Change API of ShmemAlloc() so it throws error rather than return
Date: 2016-09-01 14:14:03
Message-ID: E1bfSkl-00053y-U6@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Change API of ShmemAlloc() so it throws error rather than returning NULL.

A majority of callers seem to have believed that this was the API spec
already, because they omitted any check for a NULL result, and hence
would crash on an out-of-shared-memory failure. The original proposal
was to just add such error checks everywhere, but that does nothing to
prevent similar omissions in future. Instead, let's make ShmemAlloc()
throw the error (so we can remove the caller-side checks that do exist),
and introduce a new function ShmemAllocNoError() that has the previous
behavior of returning NULL, for the small number of callers that need
that and are prepared to do the right thing. This also lets us remove
the rather wishy-washy behavior of printing a WARNING for out-of-shmem,
which never made much sense: either the caller has a strategy for
dealing with that, or it doesn't. It's not ShmemAlloc's business to
decide whether a warning is appropriate.

The v10 release notes will need to call this out as a significant
source-code change. It's likely that it will be a bug fix for
extension callers too, but if not, they'll need to change to using
ShmemAllocNoError().

This is nominally a bug fix, but the odds that it's fixing any live
bug are actually rather small, because in general the requests
being made by the unchecked callers were already accounted for in
determining the overall shmem size, so really they ought not fail.
Between that and the possible impact on extensions, no back-patch.

Discussion: <24843(dot)1472563085(at)sss(dot)pgh(dot)pa(dot)us>

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/6c03d981a6b64ed8caaed4e94b54ef926202c9f3

Modified Files
--------------
src/backend/storage/ipc/shmem.c | 41 +++++++++++++++++++++---------------
src/backend/storage/lmgr/predicate.c | 12 -----------
src/backend/storage/lmgr/proc.c | 6 +-----
src/include/storage/shmem.h | 1 +
4 files changed, 26 insertions(+), 34 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2016-09-01 15:45:24 pgsql: Fix minor 9.4-only bug in logical decoding.
Previous Message Christian Ullrich 2016-09-01 07:03:15 Re: pgsql: Add putenv support for msvcrt from Visual Studio 2013