Re: dynamic shared memory

From: Noah Misch <noah(at)leadboat(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jim Nasby <jim(at)nasby(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dynamic shared memory
Date: 2013-09-23 19:02:01
Message-ID: 20130923190201.GA16839@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 23, 2013 at 10:43:33AM +0530, Amit Kapila wrote:
> On Mon, Sep 23, 2013 at 12:34 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > On Sun, Sep 22, 2013 at 01:17:52PM +0530, Amit Kapila wrote:
> >> #ifdef WIN32
> >> /* use CRLF line endings on Windows */
> >> _setmode(_fileno(fh), _O_TEXT);
> >> #endif
> >
> > I suspect that call (in logfile_open()) has no effect. The file is already in
> > text mode.
>
> Won't this be required when we have to open a new file due to log
> rotation based on time?
>
> The basic point, I wanted to make is that until you use _O_TEXT mode
> explicitly, the problem LF<=>CRLF will not happen. CreateFile() API
> which is used for windows implementation of open doesn't take any
> parameter which specifies it as text or binary, only by using
> _setmode, we need to set the file mode as Text or Binary.

You are indeed correct. I had assumed that pgwin32_open() does not change the
usual Windows open()/fopen() behavior concerning line endings. No code
comment mentions otherwise, and that would make pro forma our pervasive use of
PG_BINARY. Nonetheless, it behaves as you say. I wonder if that was
intentional, and I wonder if the outcome varies between Visual Studio versions
(I tested with VS2010).

> I checked fcntl.h where there is below comment above definition of
> _O_TEXT and _O_BINARY which again is pointing to what I said above.
> /* O_TEXT files have <cr><lf> sequences translated to <lf> on read()'s,
> ** and <lf> sequences translated to <cr><lf> on write()'s
> */

However, O_TEXT is the default in a normal Windows program:
http://msdn.microsoft.com/en-us/library/ktss1a9b.aspx

> I think to be on safe side we can use PG_BINARY, but it would be
> better if we are sure that this problem can occur then only we should
> use it.
> If you think cross platform backup's can create such issues, then I
> can once test this as well.

I don't know whether writing it as binary will help or hurt that situation.
If nothing else, binary gives you one less variation to think about when
studying the code. Anyone sophisticated enough to meaningfully examine the
file will have no trouble dealing with either line ending convention.

Thanks,
nm

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-09-23 19:04:43 Re: File_fdw documentation patch to clarify OPTIONS clause
Previous Message Robert Haas 2013-09-23 18:46:39 Re: record identical operator