Hi,
On Thu, Oct 08, 2026 at 09:22:11AM -0700, Bharath Rupireddy wrote:
> Hi,
>
> On Wed, Oct 7, 2026 at 11:55 PM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > As mentioned upthread [1], I think XLogReaderFree() should also check whether
> > seg.ws_file >= 0.
> >
> > Please find attached a small patch doing that.
> >
> > [1]: https://postgr.es/m/asNr203eue0R4zpZ@bdtpg
>
> Thanks for sending the patch. I don't think we ever receive or set the
> ws_file as a non-negative integer other than -1 (neither from the
> BasicOpenFilePerm nor from the core's segment_close callbacks), so !=
> -1 or >=0 to mean that it is holding the valid fd are correct. Yes, an
> external xlogreader can set it to -2 (for example) to mean invalid fd
> in their segment_close callback, but I don't think we have anyone
> doing that.
Right, there is no issue for the current in core callbacks. That said, the comment
above segment_close() says that ws_file shall be set to a negative number, not
specifically -1. So an external callback using -2 would follow the documented behavior,
but XLogReaderFree() would invoke segment_close() again.
Using >= 0 also matches the other ws_file checks in xlogreader.c.
> PS: I looked at the slru_io.c which has a mix of both != -1 and >= 0
> for file descriptors, maybe leaving it as-is in xlogreader.c is fine.
I don't think this is the same case. In slru_io.c, fd is initialized or reset to
-1 (otherwise, it holds a valid file descriptor). So != -1 and >= 0 are equivalent
there.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
