Re: Possible NULL dereferencing (src/backend/tcop/pquery.c)

Em sex., 26 de jun. de 2020 às 18:24, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> escreveu:

> Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> writes:
> > 1.Assertion check
> > /* Caller messed up if we have neither a ready query nor held data. */
> > Assert(queryDesc || portal->holdStore);
>
> > But in release, if QueryDesc is NULL and portal->holdStore is NULL too,
> > when Call PushActiveSnapshot *deference* NULL check can happen.
>
> > 2. if (portal->atEnd || count <= 0) is True
> > No need to recheck count against FETCH_ALL.
>
> > Is it worth correcting them?
>
> No.
>
> The assertion already says that that's a case that cannot happen.
> Or to look at it another way: if the case were to occur in a devel
> build, you'd get a core dump at the assertion. If the case were
> to occur in a production build, you'd get a core dump at the
> dereference. Not much difference. Either way, it's a *caller*
> bug, because the caller is supposed to make sure this cannot happen.
> If we thought that it could possibly happen, we would use an ereport
> but not an assertion; having both for the same condition is quite
> misguided.
>
Ok, thats a job of Assertion.
But I still worry that, in some rare cases, portal-> holdStore might be
corrupted in some way
and the function is called, causing a segmentation fault.

>
> (If Coverity is whining about this for you, there's something wrong
> with your Coverity settings. In the project's instance, Coverity
> accepts assertions as assertions.)
>
Probable, because reports this:
CID 10127 (#2 of 2): Dereference after null check (FORWARD_NULL)8.
var_deref_op: Dereferencing null pointer queryDesc.

>
> I'm unimpressed with the other proposed change too; it's making the logic
> more complicated and fragile for a completely negligible "performance
> gain". Moreover the compiler could probably make the same optimization.
>
Ok.

Anyway, thank you for by responding, your observations are always valuable
and help learn "the postgres way" to develop.
It's not easy.

best regards,
Ranier Vilela