Re: dynamic shared memory - Mailing list pgsql-hackers

On Tue, Sep 24, 2013 at 12:32 AM, Noah Misch <noah@leadboat.com> wrote:
> 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@leadboat.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.

The only form of comment to give an indication (or atleast I got the
indication from there) is what I have mentioned in above mail chain
at top of PG_BINARY. I understand that it is not very clear in that
comment about the actual handling of CRLF issue.

> 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).

Ideally it should not the depend on VS version as outcome depends on
API (CreateFile/setmode) usage.

>> 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.

In that case, shouldn't all other places be consistent. One reason I
had in mind for
using appropriate mode is that somebody reading code can tomorrow come
up with a question or a
patch to use correct mode, then we will again be in same situation.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com