Re: AIO v2.5
От | Andres Freund |
---|---|
Тема | Re: AIO v2.5 |
Дата | |
Msg-id | hitjys36tpaabli42imk3c2xdcgo6nt4fdj7w4ri4uznj5prx2@lllwci2cz5ng обсуждение исходный текст |
Ответ на | Re: AIO v2.5 (Matthias van de Meent <boekewurm+postgres@gmail.com>) |
Список | pgsql-hackers |
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
В списке pgsql-hackers по дате отправления: