Horton

Re: Check for tuplestorestate nullness before dereferencing

Nikolay Shaplov <dhyan@nataraj.su>

View thread
From: Nikolay Shaplov <dhyan@nataraj.su>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alena Rybakina <a.rybakina@postgrespro.ru>, pgsql-hackers@lists.postgresql.org, Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>, kuznetsovam@altlinux.org, nickel@altlinux.org, egori@altlinux.org, David Rowley <dgrowleyml@gmail.com>
Date: 2025-04-22T16:11:51Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Create infrastructure for 'MinimalTuple' representation of in-memory

  2. Teach nodeSort and nodeMaterial to optimize out unnecessary overhead

В письме от вторник, 22 апреля 2025 г. 18:50:49 MSK пользователь Tom Lane 
написал:

> The reason that the subsequent bit of code is safe is that !forward
> should not possibly be true unless EXEC_FLAG_BACKWARD was given,
> which'd cause us to create a tuplestore.  So if we were going
> to change anything, I'd propose adding something more like
> 
>         if (!forward && eof_tuplestore)
>         {
> +               Assert(node->eflags & EXEC_FLAG_BACKWARD);
>                 if (!node->eof_underlying)
>                 {
>                         /*
> 
> or perhaps more directly, Assert that tuplestore is not null.

I like  Assert(node->eflags & EXEC_FLAG_BACKWARD);
solution, it gives more information: "here we expect that BACKWARD eflag is 
set". (I am not quite familiar with this part of code, this knowledge in not 
obvious for me)
While Assert(tuplestorestate != NULL)  gives much less information about what 
is happening here.

And I think it is better to add this Assert there. People will continue using 
static analyzers on postgres code, finding this place again and again. Better 
to close this issue once and for all :-)

-- 
Nikolay Shaplov aka Nataraj
Fuzzing Engineer at Postgres Professional
Matrix IM: @dhyan:nataraj.su