Re: shm_toc_lookup API

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-06-05T15:36:40Z
Lists: pgsql-hackers
On Mon, Jun 5, 2017 at 11:19 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> shm_toc_lookup() returns NULL if it fails to find the desired key.
> Out of the 30 or so call sites, there is exactly one that has any
> use for that.  Some of the rest have Asserts that they get back
> a non-null result, but the majority just blithely dereference the
> pointer.  I do not find this cool at all; given that we're accessing
> a shared data structure, we should assign more than zero probability
> to the idea that we might not find what we expect to find when we
> expect to find it.
>
> I think that shm_toc_lookup() ought to be made to throw elog(ERROR) on an
> unexpected failure.  To satisfy the one caller that doesn't want an error,
> we could either add a "bool noError" parameter to it, or split it into two
> functions shm_toc_lookup() and shm_toc_lookup_noerror().  The latter would
> require touching less code, but the former is probably more like what we'd
> have had if this were designed more carefully to begin with.

Either is OK with me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Commits

  1. Code review for shm_toc.h/.c.

  2. Don't be so trusting that shm_toc_lookup() will always succeed.