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

On Tue, 19 Apr 2022 at 22:21, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Japin Li <japinli(at)hotmail(dot)com> writes:
>> On Tue, 19 Apr 2022 at 14:20, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> I think the comment's at best misleading. See e.g. 66f8687a8.
>>> It might be okay to use "rb" to read a text file when there
>>> is actually \r-stripping logic present, but you need to check
>>> that. Using "wb" to write a text file is flat wrong.
>
>> Thanks for the detail explanation. Should we remove the misleading comment?
>
> We should rewrite it, not just remove it. But I'm not 100% sure
> what to say instead. I wonder whether the comment's claims about
> control-Z processing still apply on modern Windows.
>

It might be true [1].

[1] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-170

> Another question is whether we actually like the current shape of
> the code. I can see at least two different directions we might
> prefer to the status quo:
>
> * Invent '#define PG_TEXT_R "r"' and so on, and use those in the
> calls that currently use plain "r" etc, establishing a project
> policy that you should use one of these six macros and never the
> underlying strings directly. This perhaps has some advantages
> in greppability and clarity of intent, but I can't help wondering
> if it's mostly obsessive-compulsiveness.
>
> * In the other direction, decide that the PG_BINARY_X macros are
> offering no benefit at all and just rip 'em out, writing "rb" and
> so on in their place. POSIX specifies that the character "b" has
> no effect on Unix-oid systems, and it has said that for thirty years
> now, so we do not really need the platform dependency that presently
> exists in the macro definitions. The presence or absence of "b"
> would serve fine as an indicator of intent, and there would be one
> less PG-specific coding convention to remember.
>

I'm incline the second direction if we need to change this.

> Or maybe it's fine as-is. Any sort of wide-ranging change like this
> creates hazards for back-patching, so we shouldn't do it lightly.
>

Agreed. Thanks again for the explanation.

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