Re: How about renaming XLogFileRead() to XLogFileOpenForRead() and XLogFileOpen() to XLogFileOpenForWrite()?

On Tue, May 10, 2022 at 6:16 PM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>
> Hi All,
>
> Currently, in postgres we have two different functions that are
> specially used to open the WAL files for reading and writing purposes.
> The first one is XLogFileOpen() that is just used to open the WAL file
> so that we can write WAL data in it. And then we have another function
> named XLogFileRead that does the same thing but is used when reading
> the WAL files during recovery time. How about renaming the function
> XLogFileRead to XLogFileOpenForRead and the other one can be renamed
> to XLogFileOpenForWrite. I think it will make the function name more
> clear and increase the readability. At least XlogFileRead doesn't look
> good to me, from the function name it actually appears like we are
> trying to read a WAL file here but actually we are opening it so that
> it can be read by some other routine.
>
> Also I see that we are passing emode to the XLogFileRead function
> which is not being used anywhere in the function, so can we remove it?

Renaming XLogFileOpen to XLogFileOpenForWrite while it uses O_RDWR,
not O_RDWR is sort of conflicting. Also, I'm concerned that
XLogFileOpen is an extern function, the external modules using it
might break. XLogFileRead uses O_RDONLY and is a static function, so
it might be okay to change the name, the only concern is that it
creates diff with the older versions as we usually don't backport
renaming functions or variables/code improvements/not-so-critical
changes.

Having said that, IMHO, the existing functions and their names look
fine to me (developers can read the function/function comments to
understand their usage though).

Regards,
Bharath Rupireddy.