Hi,
On 2025-07-10 21:00:21 +0200, Matthias van de Meent wrote:
> On Wed, 9 Jul 2025 at 16:59, Andres Freund <andres@anarazel.de> wrote:
> > On 2025-07-09 13:26:09 +0200, Matthias van de Meent wrote:
> > > I've been going through the new AIO code as an effort to rebase and
> > > adapt Neon to PG18. In doing so, I found the following
> > > items/curiosities:
> > >
> > > 1. In aio/README.md, the following code snippet is found:
> > >
> > > [...]
> > > pgaio_io_set_handle_data_32(ioh, (uint32 *) buffer, 1);
> > > [...]
> > >
> > > I believe it would be clearer if it took a reference to the buffer:
> > >
> > > pgaio_io_set_handle_data_32(ioh, (uint32 *) &buffer, 1);
> > >
> > > The main reason here is that common practice is to have a `Buffer
> > > buffer;` whereas a Buffer * is more commonly plural.
> >
> > It's also just simply wrong as-is :/. Interpreting the buffer id as a pointer
> > obviously makes no sense...
>
> Given that the snippet didn't contain type indications for buffer upto
> that point, technically the buffer variable could've been defined as
> `Buffer* buffer;` which would've been type-correct. That would be very
> confusing however, hence the suggested change.
>
> After your mail, I also noticed the later snippet which should be updated, too:
>
> ```
> -smgrstartreadv(ioh, operation->smgr, forknum, blkno,
> - BufferGetBlock(buffer), 1);
> +void *page = BufferGetBlock(buffer);
> +smgrstartreadv(ioh, operation->smgr, forknum, blkno,
> + &page, 1);
> ```
I now pushed your change, with that squashed in.
Thanks!
Andres
