Thread: use binary mode on syslog pipe on windows to avoid upsetting chunking protocol
use binary mode on syslog pipe on windows to avoid upsetting chunking protocol
From
Andrew Dunstan
Date:
This small patch makes the syslog pipe use binary mode on Windows so
that CRLF translation (and possibly other oddities) don't upset the pipe
chunking protocol. It preserves text mode for the redirected syslog
file, as recently discussed on -hackers, so there should be no visible
change.
If there is no objection I will apply this and backport it to 8.0 shortly.
cheers
andrew
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.535
diff -c -r1.535 postmaster.c
*** src/backend/postmaster/postmaster.c 24 Jul 2007 04:54:09 -0000 1.535
--- src/backend/postmaster/postmaster.c 30 Jul 2007 12:30:28 -0000
***************
*** 3385,3390 ****
--- 3385,3394 ----
MyProcPid = getpid(); /* reset MyProcPid */
+ #ifdef WIN32
+ _setmode(fileno(stderr),_O_BINARY);
+ #endif
+
/* Lose the postmaster's on-exit routines (really a no-op) */
on_exit_reset();
Index: src/backend/postmaster/syslogger.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/syslogger.c,v
retrieving revision 1.33
diff -c -r1.33 syslogger.c
*** src/backend/postmaster/syslogger.c 19 Jul 2007 19:13:43 -0000 1.33
--- src/backend/postmaster/syslogger.c 30 Jul 2007 12:30:28 -0000
***************
*** 162,167 ****
--- 162,171 ----
MyProcPid = getpid(); /* reset MyProcPid */
+ #ifdef WIN32
+ _setmode(_fileno(stderr),_O_TEXT);
+ #endif
+
#ifdef EXEC_BACKEND
syslogger_parseArgs(argc, argv);
#endif /* EXEC_BACKEND */
***************
*** 533,544 ****
fflush(stderr);
fd = _open_osfhandle((long) syslogPipe[1],
! _O_APPEND | _O_TEXT);
if (dup2(fd, _fileno(stderr)) < 0)
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not redirect stderr: %m")));
close(fd);
/* Now we are done with the write end of the pipe. */
CloseHandle(syslogPipe[1]);
syslogPipe[1] = 0;
--- 537,549 ----
fflush(stderr);
fd = _open_osfhandle((long) syslogPipe[1],
! _O_APPEND | _O_BINARY);
if (dup2(fd, _fileno(stderr)) < 0)
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not redirect stderr: %m")));
close(fd);
+ _setmode(_fileno(stderr),_O_BINARY);
/* Now we are done with the write end of the pipe. */
CloseHandle(syslogPipe[1]);
syslogPipe[1] = 0;
***************
*** 626,632 ****
fd = atoi(*argv++);
if (fd != 0)
{
! fd = _open_osfhandle(fd, _O_APPEND);
if (fd > 0)
{
syslogFile = fdopen(fd, "a");
--- 631,637 ----
fd = atoi(*argv++);
if (fd != 0)
{
! fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
if (fd > 0)
{
syslogFile = fdopen(fd, "a");
***************
*** 990,995 ****
--- 995,1001 ----
/* On Windows, need to interlock against data-transfer thread */
#ifdef WIN32
+ _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
EnterCriticalSection(&sysfileSection);
#endif
fclose(syslogFile);
Re: use binary mode on syslog pipe on windows to avoid upsetting chunking protocol
From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> This small patch makes the syslog pipe use binary mode on Windows
You need to think harder about where you are inserting the additions,
as these choices seem a bit random. For instance here:
> /* On Windows, need to interlock against data-transfer thread */
> #ifdef WIN32
> + _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
> EnterCriticalSection(&sysfileSection);
> #endif
> fclose(syslogFile);
you have inserted a 100% unrelated action between a comment and the
statement it describes. How readable will this code be to the next
person? Far better to expend an additional #ifdef/#endif pair, if
needed, to put the call either by itself or with some code that it's
sensibly related to. Being "WIN32" is not sufficient connection.
regards, tom lane
Re: use binary mode on syslog pipe on windows to avoid upsetting chunking protocol
From
Andrew Dunstan
Date:
Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> This small patch makes the syslog pipe use binary mode on Windows
>>
>
> You need to think harder about where you are inserting the additions,
> as these choices seem a bit random.
>
Is this more to your taste?
cheers
andrew
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.535
diff -c -r1.535 postmaster.c
*** src/backend/postmaster/postmaster.c 24 Jul 2007 04:54:09 -0000 1.535
--- src/backend/postmaster/postmaster.c 30 Jul 2007 15:52:57 -0000
***************
*** 3385,3390 ****
--- 3385,3399 ----
MyProcPid = getpid(); /* reset MyProcPid */
+ /* make sure stderr is in binary mode before anything can
+ * possibly be written to it, in case it's actually the syslogger pipe,
+ * so the pipe chunking protocol isn't disturbed. Non-logpipe data
+ * gets translated on redirection (e.g. via pg_ctl -l) anyway.
+ */
+ #ifdef WIN32
+ _setmode(fileno(stderr),_O_BINARY);
+ #endif
+
/* Lose the postmaster's on-exit routines (really a no-op) */
on_exit_reset();
***************
*** 3396,3401 ****
--- 3405,3412 ----
MemoryContextInit();
InitializeGUCOptions();
+
+
/* Read in the variables file */
memset(&port, 0, sizeof(Port));
read_backend_variables(argv[2], &port);
Index: src/backend/postmaster/syslogger.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/syslogger.c,v
retrieving revision 1.33
diff -c -r1.33 syslogger.c
*** src/backend/postmaster/syslogger.c 19 Jul 2007 19:13:43 -0000 1.33
--- src/backend/postmaster/syslogger.c 30 Jul 2007 15:52:57 -0000
***************
*** 194,199 ****
--- 194,208 ----
close(fd);
}
+ /* Syslogger's own stderr can't be the syslogPipe, so set it back to
+ * text mode if we didn't just close it.
+ * (It was set to binary in SubPostmasterMain).
+ */
+ #ifdef WIN32
+ else
+ _setmode(_fileno(stderr),_O_TEXT);
+ #endif
+
/*
* Also close our copy of the write end of the pipe. This is needed to
* ensure we can detect pipe EOF correctly. (But note that in the restart
***************
*** 531,544 ****
#else
int fd;
fflush(stderr);
fd = _open_osfhandle((long) syslogPipe[1],
! _O_APPEND | _O_TEXT);
if (dup2(fd, _fileno(stderr)) < 0)
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not redirect stderr: %m")));
close(fd);
/* Now we are done with the write end of the pipe. */
CloseHandle(syslogPipe[1]);
syslogPipe[1] = 0;
--- 540,559 ----
#else
int fd;
+ /*
+ * open the pipe in binary mode and make sure
+ * stderr is binary after it's been dup'ed into, to avoid
+ * disturbing the pipe chunking protocol.
+ */
fflush(stderr);
fd = _open_osfhandle((long) syslogPipe[1],
! _O_APPEND | _O_BINARY);
if (dup2(fd, _fileno(stderr)) < 0)
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not redirect stderr: %m")));
close(fd);
+ _setmode(_fileno(stderr),_O_BINARY);
/* Now we are done with the write end of the pipe. */
CloseHandle(syslogPipe[1]);
syslogPipe[1] = 0;
***************
*** 626,632 ****
fd = atoi(*argv++);
if (fd != 0)
{
! fd = _open_osfhandle(fd, _O_APPEND);
if (fd > 0)
{
syslogFile = fdopen(fd, "a");
--- 641,647 ----
fd = atoi(*argv++);
if (fd != 0)
{
! fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
if (fd > 0)
{
syslogFile = fdopen(fd, "a");
***************
*** 988,993 ****
--- 1003,1012 ----
setvbuf(fh, NULL, LBF_MODE, 0);
+ #ifdef WIN32
+ _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
+ #endif
+
/* On Windows, need to interlock against data-transfer thread */
#ifdef WIN32
EnterCriticalSection(&sysfileSection);
Re: use binary mode on syslog pipe on windows to avoid upsetting chunking protocol
From
Andrew Dunstan
Date:
Andrew Dunstan wrote: > > > Tom Lane wrote: >> Andrew Dunstan <andrew@dunslane.net> writes: >> >>> This small patch makes the syslog pipe use binary mode on Windows >>> >> >> You need to think harder about where you are inserting the additions, >> as these choices seem a bit random. > > Is this more to your taste? > > ... and yes I'll fix the extra lines :-) cheers andrew
Re: use binary mode on syslog pipe on windows to avoid upsetting chunking protocol
From
Andrew Dunstan
Date:
Andrew Dunstan wrote: > > >>> >>>> This small patch makes the syslog pipe use binary mode on Windows >>>> >>> > This is now committed and ported back to 8.0. cheers andrew