Re: Replace open mode with PG_BINARY_R/W/A macros

On Tue, 19 Apr 2022 at 14:14, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Tue, Apr 19, 2022 at 01:29:18PM +0800, Japin Li wrote:
>> On Mon, 18 Apr 2022 at 22:41, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Japin Li <japinli(at)hotmail(dot)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.

Oh, I understand your points. Thanks for the explanation.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.