Re: Replace open mode with PG_BINARY_R/W/A macros - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Replace open mode with PG_BINARY_R/W/A macros
Date
Msg-id 2548264.1650378097@sss.pgh.pa.us
Whole thread Raw
In response to Re: Replace open mode with PG_BINARY_R/W/A macros  (Japin Li <japinli@hotmail.com>)
Responses Re: Replace open mode with PG_BINARY_R/W/A macros  (Japin Li <japinli@hotmail.com>)
Re: Replace open mode with PG_BINARY_R/W/A macros  (Peter Eisentraut <peter.eisentraut@enterprisedb.com>)
List pgsql-hackers
Japin Li <japinli@hotmail.com> writes:
> On Tue, 19 Apr 2022 at 14:20, Tom Lane <tgl@sss.pgh.pa.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.

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.

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.

            regards, tom lane



pgsql-hackers by date:

Previous
From: "David G. Johnston"
Date:
Subject: Re: pg14 psql broke \d datname.nspname.relname
Next
From: Niyas Sait
Date:
Subject: Re: [PATCH] Add native windows on arm64 support