Re: dynamic shared memory

On Sun, Sep 22, 2013 at 01:17:52PM +0530, Amit Kapila wrote:
> On Fri, Sep 20, 2013 at 5:14 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > On 2013-09-19 11:44:34 -0400, Robert Haas wrote:
> >> On Wed, Sep 18, 2013 at 1:42 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> >> >> + /* Create or truncate the file. */
> >> >> + statefd = open(PG_DYNSHMEM_NEW_STATE_FILE, O_RDWR|O_CREAT|O_TRUNC, 0600);
> >> >
> >> > Doesn't this need a | PG_BINARY?
> >>
> >> It's a text file. Do we need PG_BINARY anyway?
> >
> > I'd say yes. Non binary mode stuff on windows does stuff like
> > transforming LF <=> CRLF on reading/writing, which makes sizes not match
> > up and similar ugliness.
> > Imo there's little reason to use non-binary mode for anything written
> > for postgres' own consumption.
>
> On checking about this in code, I found the below comment which
> suggests LF<=> CRLF is not an issue (in windows it uses pgwin32_open
> to open a file):
>
> /*
> * NOTE: this is also used for opening text files.
> * WIN32 treats Control-Z as EOF in files opened in text mode.
> * Therefore, we open files in binary mode on Win32 so we can read
> * literal control-Z. The other affect is that we see CRLF, but
> * that is OK because we can already handle those cleanly.
> */

That comment appears at the definition of PG_BINARY. You only get what it
describes when you use PG_BINARY.

> Second instance, I noticed in code as below which again suggests CRLF
> should not be an issue until file mode is specifically set to TEXT
> mode which is not the case with current usage of open in dynamic
> shared memory code.
>
> #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.

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