Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 5b0b699

Browse files
committed
Refactor output file handling when forking syslogger under EXEC_BACKEND
A forked logging collector in EXEC_BACKEND builds passes down file descriptors (or HANDLEs in WIN32) through a command for files to be reopened (for stderr and csvlog). Some of its logic was duplicated, and this commit refactors the code with some wrapper routines for file reopening after forking and fd grabbing when building the command for the fork. While on it, this simplifies a use of "long" in the code, introduced by ab0ba6e to take care of a warning related to MinGW-W64 when mapping a intptr_t to a printed value. "long" is 32-bit long on Windows, and interoperability of Win32 and Win64 ensures that handles are always 32-bit significant, so we can just use "int" for the same result. This also makes the new routines more symmetric. This change makes easier the introduction of new log destinations in the logging collector, and this is not the only piece of refactoring planned. I have tested this change with EXEC_BACKEND on linux, macos, and of course MSVC (both Win32 and Win64), but not MinGW so the buildfarm may have something to say here. Author: Sehrope Sarkuni, Michael Paquier Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
1 parent 6bc6bd4 commit 5b0b699

File tree

1 file changed

+62
-60
lines changed

1 file changed

+62
-60
lines changed

src/backend/postmaster/syslogger.c

+62-60
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ static volatile sig_atomic_t rotation_requested = false;
130130

131131
/* Local subroutines */
132132
#ifdef EXEC_BACKEND
133+
static int syslogger_fdget(FILE *file);
134+
static FILE *syslogger_fdopen(int fd);
133135
static pid_t syslogger_forkexec(void);
134136
static void syslogger_parseArgs(int argc, char *argv[]);
135137
#endif
@@ -733,6 +735,60 @@ SysLogger_Start(void)
733735

734736
#ifdef EXEC_BACKEND
735737

738+
/*
739+
* syslogger_fdget() -
740+
*
741+
* Utility wrapper to grab the file descriptor of an opened error output
742+
* file. Used when building the command to fork the logging collector.
743+
*/
744+
static int
745+
syslogger_fdget(FILE *file)
746+
{
747+
#ifndef WIN32
748+
if (file != NULL)
749+
return fileno(file);
750+
else
751+
return -1;
752+
#else
753+
if (file != NULL)
754+
return (int) _get_osfhandle(_fileno(file));
755+
else
756+
return 0;
757+
#endif /* WIN32 */
758+
}
759+
760+
/*
761+
* syslogger_fdopen() -
762+
*
763+
* Utility wrapper to re-open an error output file, using the given file
764+
* descriptor. Used when parsing arguments in a forked logging collector.
765+
*/
766+
static FILE *
767+
syslogger_fdopen(int fd)
768+
{
769+
FILE *file = NULL;
770+
771+
#ifndef WIN32
772+
if (fd != -1)
773+
{
774+
file = fdopen(fd, "a");
775+
setvbuf(file, NULL, PG_IOLBF, 0);
776+
}
777+
#else /* WIN32 */
778+
if (fd != 0)
779+
{
780+
fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
781+
if (fd > 0)
782+
{
783+
file = fdopen(fd, "a");
784+
setvbuf(file, NULL, PG_IOLBF, 0);
785+
}
786+
}
787+
#endif /* WIN32 */
788+
789+
return file;
790+
}
791+
736792
/*
737793
* syslogger_forkexec() -
738794
*
@@ -751,34 +807,11 @@ syslogger_forkexec(void)
751807
av[ac++] = NULL; /* filled in by postmaster_forkexec */
752808

753809
/* static variables (those not passed by write_backend_variables) */
754-
#ifndef WIN32
755-
if (syslogFile != NULL)
756-
snprintf(filenobuf, sizeof(filenobuf), "%d",
757-
fileno(syslogFile));
758-
else
759-
strcpy(filenobuf, "-1");
760-
#else /* WIN32 */
761-
if (syslogFile != NULL)
762-
snprintf(filenobuf, sizeof(filenobuf), "%ld",
763-
(long) _get_osfhandle(_fileno(syslogFile)));
764-
else
765-
strcpy(filenobuf, "0");
766-
#endif /* WIN32 */
810+
snprintf(filenobuf, sizeof(filenobuf), "%d",
811+
syslogger_fdget(syslogFile));
767812
av[ac++] = filenobuf;
768-
769-
#ifndef WIN32
770-
if (csvlogFile != NULL)
771-
snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%d",
772-
fileno(csvlogFile));
773-
else
774-
strcpy(csvfilenobuf, "-1");
775-
#else /* WIN32 */
776-
if (csvlogFile != NULL)
777-
snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%ld",
778-
(long) _get_osfhandle(_fileno(csvlogFile)));
779-
else
780-
strcpy(csvfilenobuf, "0");
781-
#endif /* WIN32 */
813+
snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%d",
814+
syslogger_fdget(csvlogFile));
782815
av[ac++] = csvfilenobuf;
783816

784817
av[ac] = NULL;
@@ -807,41 +840,10 @@ syslogger_parseArgs(int argc, char *argv[])
807840
* fails there's not a lot we can do to report the problem anyway. As
808841
* coded, we'll just crash on a null pointer dereference after failure...
809842
*/
810-
#ifndef WIN32
811-
fd = atoi(*argv++);
812-
if (fd != -1)
813-
{
814-
syslogFile = fdopen(fd, "a");
815-
setvbuf(syslogFile, NULL, PG_IOLBF, 0);
816-
}
817-
fd = atoi(*argv++);
818-
if (fd != -1)
819-
{
820-
csvlogFile = fdopen(fd, "a");
821-
setvbuf(csvlogFile, NULL, PG_IOLBF, 0);
822-
}
823-
#else /* WIN32 */
824843
fd = atoi(*argv++);
825-
if (fd != 0)
826-
{
827-
fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
828-
if (fd > 0)
829-
{
830-
syslogFile = fdopen(fd, "a");
831-
setvbuf(syslogFile, NULL, PG_IOLBF, 0);
832-
}
833-
}
844+
syslogFile = syslogger_fdopen(fd);
834845
fd = atoi(*argv++);
835-
if (fd != 0)
836-
{
837-
fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
838-
if (fd > 0)
839-
{
840-
csvlogFile = fdopen(fd, "a");
841-
setvbuf(csvlogFile, NULL, PG_IOLBF, 0);
842-
}
843-
}
844-
#endif /* WIN32 */
846+
csvlogFile = syslogger_fdopen(fd);
845847
}
846848
#endif /* EXEC_BACKEND */
847849

0 commit comments

Comments
 (0)