On Tue, Apr 19, 2022 at 01:29:18PM +0800, Japin Li wrote:
> On Mon, 18 Apr 2022 at 22:41, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Japin Li <japinli@hotmail.com> writes:
>>> I found we defined PG_BINARY_R/W/A macros for opening files, however,
>>> there are some places use the constant strings. IMO we should use
>>> those macros instead of constant strings. Here is a patch for it.
>>> Any thoughts?
>>
>> A lot of these changes look wrong to me: they are substituting "rb" for
>> "r", etc, in places that mean to read text files. You have to think
>> about the Windows semantics.
This reminded me of the business from a couple of years ago in
pgwin32_open() to enforce the text mode in the frontend if O_BINARY is
not specified.
> I do this substituting, since the comment says it can be used for opening
> text files. Maybe I misunderstand the comment.
'b' is normally ignored on POSIX platforms (per the Linux man page for
fopen), but your patch has as effect to silently switch to binary mode
on Windows all those code paths. See _setmode() in pgwin32_open(),
that changes the behavior of CRLF when reading or writing such files,
as described here:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/setmode?view=msvc-170
The change in adminpack.c would be actually as 'b' should be ignored
on non-WIN32, but Tom's point is to not take lightly all the others.
--
Michael
