On Tue, 19 Apr 2022 at 14:14, Michael Paquier <mich...@paquier.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 <t...@sss.pgh.pa.us> wrote: >>> Japin Li <japi...@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.
Oh, I understand your points. Thanks for the explanation. -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.