Re: pgsql: Fix another instance of unsafe coding for shm_toc_lookup failure - Mailing list pgsql-committers

From Thomas Munro
Subject Re: pgsql: Fix another instance of unsafe coding for shm_toc_lookup failure
Date
Msg-id CAEepm=2afhhpgK+11xZWiApaJJ_P=624ZqZP_pM2JSuLVGxGxA@mail.gmail.com
Whole thread Raw
In response to pgsql: Fix another instance of unsafe coding for shm_toc_lookupfailure  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: pgsql: Fix another instance of unsafe coding for shm_toc_lookup failure
List pgsql-committers
On Sat, Feb 3, 2018 at 12:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Fix another instance of unsafe coding for shm_toc_lookup failure.
>
> One or another author of commit 5bcf389ec seems to have thought that
> computing an offset from a NULL pointer would yield another NULL pointer.
> There may possibly be architectures where that works, but common machines
> don't work like that.  Per a quick code review of places calling
> shm_toc_lookup and not using noError = false.

Hmm... That was me.  FWIW I certainly didn't think that about pointer
arithmetic... I think I must have got confused about the sense of that
flag.  ExecHashInitializeWorker() should always find a TOC entry using
plan_node_id, because ExecHashInitializeDSM() always inserts one, so
my mistake was actually to put noError = true there when noError =
false was called for.  However, it's not surprising that you drew the
opposite conclusion (ie that it might in fact not be in the TOC),
since the shm space is really only necessary for EXPLAIN ANALYZE.
Perhaps I should make it skip setting up this shm stuff if
!node->ss.ps.instrument, just like the equivalent Sort node code.  I
will look into that on Monday.

-- 
Thomas Munro
http://www.enterprisedb.com


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Be more wary about shm_toc_lookup failure.
Next
From: Tom Lane
Date:
Subject: Re: pgsql: Fix another instance of unsafe coding for shm_toc_lookup failure