Re: Confine vacuum skip logic to lazy_scan_skip

Em qui., 27 de fev. de 2025 às 14:49, Andres Freund <andres(at)anarazel(dot)de>
escreveu:

> Hi,
>
> On 2025-02-27 12:44:24 -0500, Andres Freund wrote:
> > > CID 1592454: (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
> > > 8. var_deref_op: Dereferencing null pointer per_buffer_data.
> >
> > That's exactly what the messages you quoted are discussing, no?
>
> Ah, no, it isn't. But I still think the coverity alert and the patch don't
> make sense, as per the below:
>
> > > diff --git a/src/backend/storage/aio/read_stream.c
> b/src/backend/storage/aio/read_stream.c
> > > index 04bdb5e6d4..18e9b4f3c4 100644
> > > --- a/src/backend/storage/aio/read_stream.c
> > > +++ b/src/backend/storage/aio/read_stream.c
> > > @@ -666,6 +666,8 @@ read_stream_next_buffer(ReadStream *stream, void
> **per_buffer_data)
> > >
> READ_BUFFERS_ISSUE_ADVICE : 0)))
> > > {
> > > /* Fast return. */
> > > + if (per_buffer_data)
> > > + *per_buffer_data =
> get_per_buffer_data(stream, oldest_buffer_index);
> > > return buffer;
> > > }
> >
> > A few lines above:
> > Assert(stream->per_buffer_data_size == 0);
> >
> > The fast path isn't used when per buffer data is used. Adding a check
> for
> > per_buffer_data and assigning something to it is nonsensical.
>
Perhaps.

But the fast path and the parameter void **per_buffer_data,
IMHO, is a serious risk in my opinion.
Of course, when in runtime.

best regards,
Ranier Vilela