Re: [BUGS] BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode

From: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: henry_boehlert(at)agilent(dot)com, "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [BUGS] BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode
Date: 2017-07-14 02:27:04
Message-ID: CAJrrPGdogN7GPyU+yWXH9gTMAhOBM4nmWBLq6i4jo-n+aYo74A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Fri, Jul 14, 2017 at 2:54 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

> On 05/03/2017 07:32 AM, Haribabu Kommi wrote:
>
>> [Adding -hackers mailing list]
>>
>> On Fri, Apr 28, 2017 at 6:28 PM, <henry_boehlert(at)agilent(dot)com> wrote:
>>
>> The following bug has been logged on the website:
>>>
>>> Bug reference: 14634
>>> Logged by: Henry Boehlert
>>> Email address: henry_boehlert(at)agilent(dot)com
>>> PostgreSQL version: 9.6.2
>>> Operating system: Windows Server 2012 R2 6.3.9600
>>> Description:
>>>
>>> Executing command pg_basebackup -D -F t writes its output to stdout,
>>> which
>>> is open in text mode, causing LF to be converted to CR LF thus corrupting
>>> the resulting archive.
>>>
>>> To write the tar to stdout, on Windows stdout's mode should be
>>> temporarily
>>> switched to binary.
>>>
>>> https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx
>>>
>>>
>> Thanks for reporting the issue.
>> With the attached patch, I was able to extract the tar file that gets
>> generated when the tar file is written into stdout. I tested the
>> the compressed tar also.
>>
>> This bug needs to be fixed in back branches also.
>>
>
> Seems reasonable. One question:
>
> In the patch, you used "_setmode" function, while the calls in
> src/bin/pg_dump/pg_backup_archiver.c use "setmode". There are a few
> places in the backend that also use "_setmode". What's the difference?
> Should we change some of them to be consistent?
>

Actually there is no functional difference between these two functions.
one is a POSIX variant and another one is ISO C++ variant [1]. The support
of POSIX variant is deprecated in windows, because of this reason we should
use the _setmode instead of setmode.

I attached the patch to change the pg_dump code to use _setmode function
instead of _setmode to be consistent with other functions.

[1] -
https://docs.microsoft.com/en-gb/cpp/c-runtime-library/reference/posix-setmode

Regards,
Hari Babu
Fujitsu Australia

Attachment Content-Type Size
0002-Replace-setmode-with-_setmode-function.patch application/octet-stream 1.1 KB
0001-pg_basebackup-windows-tar-mode-to-stdout-fix.patch application/octet-stream 1.1 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2017-07-14 08:34:24 Re: PgFDW connection invalidation by ALTER SERVER/ALTER USER MAPPING
Previous Message Heikki Linnakangas 2017-07-13 16:54:10 Re: [BUGS] BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode

Browse pgsql-hackers by date

  From Date Subject
Next Message Chapman Flack 2017-07-14 02:46:46 Re: SCRAM auth and Pgpool-II
Previous Message Tatsuo Ishii 2017-07-14 01:54:38 Re: SCRAM auth and Pgpool-II