Re: Confine vacuum skip logic to lazy_scan_skip - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Confine vacuum skip logic to lazy_scan_skip
Date
Msg-id CAEudQAokdB=SynHwduW4-DWKmwxHt36e736pBPp-L+5QVapcCg@mail.gmail.com
Whole thread Raw
In response to Re: Confine vacuum skip logic to lazy_scan_skip  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
Em qui., 27 de fev. de 2025 às 14:49, Andres Freund <andres@anarazel.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

pgsql-hackers by date:

Previous
From: "David G. Johnston"
Date:
Subject: Re: Docs for pg_basebackup needs v17 note for incremental backup
Next
From: Tom Lane
Date:
Subject: Re: Confine vacuum skip logic to lazy_scan_skip