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

diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index bca38835726a508c3fd8b967dacd106e54441267..c5f9c5202dae5202f612a48b6796cb85a494ec22 100644 (file)
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -130,6 +130,8 @@ static volatile sig_atomic_t rotation_requested = false;
 
 /* Local subroutines */
 #ifdef EXEC_BACKEND
+static int syslogger_fdget(FILE *file);
+static FILE *syslogger_fdopen(int fd);
 static pid_t syslogger_forkexec(void);
 static void syslogger_parseArgs(int argc, char *argv[]);
 #endif
@@ -733,6 +735,60 @@ SysLogger_Start(void)
 
 #ifdef EXEC_BACKEND
 
+/*
+ * syslogger_fdget() -
+ *
+ * Utility wrapper to grab the file descriptor of an opened error output
+ * file.  Used when building the command to fork the logging collector.
+ */
+static int
+syslogger_fdget(FILE *file)
+{
+#ifndef WIN32
+   if (file != NULL)
+       return fileno(file);
+   else
+       return -1;
+#else
+   if (file != NULL)
+       return (int) _get_osfhandle(_fileno(file));
+   else
+       return 0;
+#endif                         /* WIN32 */
+}
+
+/*
+ * syslogger_fdopen() -
+ *
+ * Utility wrapper to re-open an error output file, using the given file
+ * descriptor.  Used when parsing arguments in a forked logging collector.
+ */
+static FILE *
+syslogger_fdopen(int fd)
+{
+   FILE       *file = NULL;
+
+#ifndef WIN32
+   if (fd != -1)
+   {
+       file = fdopen(fd, "a");
+       setvbuf(file, NULL, PG_IOLBF, 0);
+   }
+#else                          /* WIN32 */
+   if (fd != 0)
+   {
+       fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
+       if (fd > 0)
+       {
+           file = fdopen(fd, "a");
+           setvbuf(file, NULL, PG_IOLBF, 0);
+       }
+   }
+#endif                         /* WIN32 */
+
+   return file;
+}
+
 /*
  * syslogger_forkexec() -
  *
@@ -751,34 +807,11 @@ syslogger_forkexec(void)
    av[ac++] = NULL;            /* filled in by postmaster_forkexec */
 
    /* static variables (those not passed by write_backend_variables) */
-#ifndef WIN32
-   if (syslogFile != NULL)
-       snprintf(filenobuf, sizeof(filenobuf), "%d",
-                fileno(syslogFile));
-   else
-       strcpy(filenobuf, "-1");
-#else                          /* WIN32 */
-   if (syslogFile != NULL)
-       snprintf(filenobuf, sizeof(filenobuf), "%ld",
-                (long) _get_osfhandle(_fileno(syslogFile)));
-   else
-       strcpy(filenobuf, "0");
-#endif                         /* WIN32 */
+   snprintf(filenobuf, sizeof(filenobuf), "%d",
+            syslogger_fdget(syslogFile));
    av[ac++] = filenobuf;
-
-#ifndef WIN32
-   if (csvlogFile != NULL)
-       snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%d",
-                fileno(csvlogFile));
-   else
-       strcpy(csvfilenobuf, "-1");
-#else                          /* WIN32 */
-   if (csvlogFile != NULL)
-       snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%ld",
-                (long) _get_osfhandle(_fileno(csvlogFile)));
-   else
-       strcpy(csvfilenobuf, "0");
-#endif                         /* WIN32 */
+   snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%d",
+            syslogger_fdget(csvlogFile));
    av[ac++] = csvfilenobuf;
 
    av[ac] = NULL;
@@ -807,41 +840,10 @@ syslogger_parseArgs(int argc, char *argv[])
     * fails there's not a lot we can do to report the problem anyway.  As
     * coded, we'll just crash on a null pointer dereference after failure...
     */
-#ifndef WIN32
-   fd = atoi(*argv++);
-   if (fd != -1)
-   {
-       syslogFile = fdopen(fd, "a");
-       setvbuf(syslogFile, NULL, PG_IOLBF, 0);
-   }
-   fd = atoi(*argv++);
-   if (fd != -1)
-   {
-       csvlogFile = fdopen(fd, "a");
-       setvbuf(csvlogFile, NULL, PG_IOLBF, 0);
-   }
-#else                          /* WIN32 */
    fd = atoi(*argv++);
-   if (fd != 0)
-   {
-       fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
-       if (fd > 0)
-       {
-           syslogFile = fdopen(fd, "a");
-           setvbuf(syslogFile, NULL, PG_IOLBF, 0);
-       }
-   }
+   syslogFile = syslogger_fdopen(fd);
    fd = atoi(*argv++);
-   if (fd != 0)
-   {
-       fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
-       if (fd > 0)
-       {
-           csvlogFile = fdopen(fd, "a");
-           setvbuf(csvlogFile, NULL, PG_IOLBF, 0);
-       }
-   }
-#endif                         /* WIN32 */
+   csvlogFile = syslogger_fdopen(fd);
 }
 #endif                         /* EXEC_BACKEND */