Windows: Make pg_ctl reliably detect service status

pg_ctl is using isatty() to verify whether the process is running in a
terminal, and if not it sends its output to Windows' Event Log ... which
does the wrong thing when the output has been redirected to a pipe, as
reported in bug #13592.

To fix, make pg_ctl use the code we already have to detect service-ness:
in the master branch, move src/backend/port/win32/security.c to src/port
(with suitable tweaks so that it runs properly in backend and frontend
environments); pg_ctl already has access to pgport so it Just Works.  In
older branches, that's likely to cause trouble, so instead duplicate the
required code in pg_ctl.c.

Author: Michael Paquier
Bug report and diagnosis: Egon Kocjan
Backpatch: all supported branches

diff --git a/src/backend/port/win32/Makefile b/src/backend/port/win32/Makefile
index 820a3b3753ec2cd595c5a1c0e7f72fdac129d8b1..a6ace93e261a71c789b713d861d0b75c86beb4f4 100644 (file)
--- a/src/backend/port/win32/Makefile
+++ b/src/backend/port/win32/Makefile
@@ -12,7 +12,7 @@ subdir = src/backend/port/win32
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = timer.o socket.o signal.o security.o mingwcompat.o
+OBJS = timer.o socket.o signal.o mingwcompat.o
 ifeq ($(have_win32_dbghelp), yes)
 OBJS += crashdump.o
 endif

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index bd82a059a04b4e50e8a97a1af28117cb3ee432ac..28d3cf2a8f9a4250110117674932aa70a320a4dc 100644 (file)
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -216,7 +216,7 @@ write_stderr(const char *fmt,...)
     * On Win32, we print to stderr if running on a console, or write to
     * eventlog if running as a service
     */
-   if (!isatty(fileno(stderr)))    /* Running as a service */
+   if (!pgwin32_is_service())  /* Running as a service */
    {
        char        errbuf[2048];       /* Arbitrary size? */
 

diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index 4cb51ec7bea68476a25402c58dc8b2f1d0e75534..69cd1c41e154331de76e19d446774d932ca057d2 100644 (file)
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -382,9 +382,6 @@ int         pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout);
 
 extern int pgwin32_noblock;
 
-/* in backend/port/win32/security.c */
-extern int pgwin32_is_admin(void);
-extern int pgwin32_is_service(void);
 #endif
 
 /* in backend/port/win32_shmem.c */
@@ -400,6 +397,10 @@ extern void _dosmaperr(unsigned long);
 extern int pgwin32_putenv(const char *);
 extern void pgwin32_unsetenv(const char *);
 
+/* in port/win32security.c */
+extern int pgwin32_is_service(void);
+extern int pgwin32_is_admin(void);
+
 #define putenv(x) pgwin32_putenv(x)
 #define unsetenv(x) pgwin32_unsetenv(x)
 

diff --git a/src/backend/port/win32/security.c b/src/port/win32security.c
similarity index 73%
rename from src/backend/port/win32/security.c
rename to src/port/win32security.c
index 327ed404fdc5f577cce73e376bbfa5b5c1a2320c..66796e0e286bdc0e03064eea4c890db1f9753f68 100644 (file)
--- a/src/backend/port/win32/security.c
+++ b/src/port/win32security.c
@@ -1,22 +1,47 @@
 /*-------------------------------------------------------------------------
  *
- * security.c
+ * win32security.c
  *   Microsoft Windows Win32 Security Support Functions
  *
  * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *   src/backend/port/win32/security.c
+ *   src/port/win32security.c
  *
  *-------------------------------------------------------------------------
  */
 
+#ifndef FRONTEND
 #include "postgres.h"
+#else
+#include "postgres_fe.h"
+#endif
 
 
 static BOOL pgwin32_get_dynamic_tokeninfo(HANDLE token,
-                           TOKEN_INFORMATION_CLASS class, char **InfoBuffer,
-                             char *errbuf, int errsize);
+                             TOKEN_INFORMATION_CLASS class,
+                             char **InfoBuffer, char *errbuf, int errsize);
+
+
+/*
+ * Utility wrapper for frontend and backend when reporting an error
+ * message.
+ */
+static
+pg_attribute_printf(1, 2)
+void
+log_error(const char *fmt,...)
+{
+   va_list     ap;
+
+   va_start(fmt, ap);
+#ifndef FRONTEND
+   write_stderr(fmt, ap);
+#else
+   fprintf(stderr, fmt, ap);
+#endif
+   va_end(ap);
+}
 
 /*
  * Returns nonzero if the current user has administrative privileges,
@@ -40,15 +65,15 @@ pgwin32_is_admin(void)
 
    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_READ, &AccessToken))
    {
-       write_stderr("could not open process token: error code %lu\n",
-                    GetLastError());
+       log_error("could not open process token: error code %lu\n",
+                 GetLastError());
        exit(1);
    }
 
    if (!pgwin32_get_dynamic_tokeninfo(AccessToken, TokenGroups,
                                       &InfoBuffer, errbuf, sizeof(errbuf)))
    {
-       write_stderr("%s", errbuf);
+       log_error("%s", errbuf);
        exit(1);
    }
 
@@ -57,20 +82,22 @@ pgwin32_is_admin(void)
    CloseHandle(AccessToken);
 
    if (!AllocateAndInitializeSid(&NtAuthority, 2,
-        SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0,
+                                 SECURITY_BUILTIN_DOMAIN_RID,
+                                 DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0,
                                  0, &AdministratorsSid))
    {
-       write_stderr("could not get SID for Administrators group: error code %lu\n",
-                    GetLastError());
+       log_error("could not get SID for Administrators group: error code %lu\n",
+                 GetLastError());
        exit(1);
    }
 
    if (!AllocateAndInitializeSid(&NtAuthority, 2,
-   SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_POWER_USERS, 0, 0, 0, 0, 0,
+                                 SECURITY_BUILTIN_DOMAIN_RID,
+                                 DOMAIN_ALIAS_RID_POWER_USERS, 0, 0, 0, 0, 0,
                                  0, &PowerUsersSid))
    {
-       write_stderr("could not get SID for PowerUsers group: error code %lu\n",
-                    GetLastError());
+       log_error("could not get SID for PowerUsers group: error code %lu\n",
+                 GetLastError());
        exit(1);
    }
 
@@ -78,8 +105,10 @@ pgwin32_is_admin(void)
 
    for (x = 0; x < Groups->GroupCount; x++)
    {
-       if ((EqualSid(AdministratorsSid, Groups->Groups[x].Sid) && (Groups->Groups[x].Attributes & SE_GROUP_ENABLED)) ||
-           (EqualSid(PowerUsersSid, Groups->Groups[x].Sid) && (Groups->Groups[x].Attributes & SE_GROUP_ENABLED)))
+       if ((EqualSid(AdministratorsSid, Groups->Groups[x].Sid) &&
+            (Groups->Groups[x].Attributes & SE_GROUP_ENABLED)) ||
+           (EqualSid(PowerUsersSid, Groups->Groups[x].Sid) &&
+            (Groups->Groups[x].Attributes & SE_GROUP_ENABLED)))
        {
            success = TRUE;
            break;
@@ -105,9 +134,10 @@ pgwin32_is_admin(void)
  *  1 = Service
  * -1 = Error
  *
- * Note: we can't report errors via either ereport (we're called too early)
- * or write_stderr (because that calls this).  We are therefore reduced to
- * writing directly on stderr, which sucks, but we have few alternatives.
+ * Note: we can't report errors via either ereport (we're called too early
+ * in the backend) or write_stderr (because that calls this).  We are
+ * therefore reduced to writing directly on stderr, which sucks, but we
+ * have few alternatives.
  */
 int
 pgwin32_is_service(void)
@@ -217,13 +247,15 @@ pgwin32_get_dynamic_tokeninfo(HANDLE token, TOKEN_INFORMATION_CLASS class,
 
    if (GetTokenInformation(token, class, NULL, 0, &InfoBufferSize))
    {
-       snprintf(errbuf, errsize, "could not get token information: got zero size\n");
+       snprintf(errbuf, errsize,
+                "could not get token information: got zero size\n");
        return FALSE;
    }
 
    if (GetLastError() != ERROR_INSUFFICIENT_BUFFER)
    {
-       snprintf(errbuf, errsize, "could not get token information: error code %lu\n",
+       snprintf(errbuf, errsize,
+                "could not get token information: error code %lu\n",
                 GetLastError());
        return FALSE;
    }
@@ -231,7 +263,8 @@ pgwin32_get_dynamic_tokeninfo(HANDLE token, TOKEN_INFORMATION_CLASS class,
    *InfoBuffer = malloc(InfoBufferSize);
    if (*InfoBuffer == NULL)
    {
-       snprintf(errbuf, errsize, "could not allocate %d bytes for token information\n",
+       snprintf(errbuf, errsize,
+                "could not allocate %d bytes for token information\n",
                 (int) InfoBufferSize);
        return FALSE;
    }
@@ -239,7 +272,8 @@ pgwin32_get_dynamic_tokeninfo(HANDLE token, TOKEN_INFORMATION_CLASS class,
    if (!GetTokenInformation(token, class, *InfoBuffer,
                             InfoBufferSize, &InfoBufferSize))
    {
-       snprintf(errbuf, errsize, "could not get token information: error code %lu\n",
+       snprintf(errbuf, errsize,
+                "could not get token information: error code %lu\n",
                 GetLastError());
        return FALSE;
    }

diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index c122f0259c9c677b36d34d243cfa4c7d9d5344c7..1dba7d9662ee9bd5d54ebc9d8cb55b0e39b89eb1 100644 (file)
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -90,7 +90,7 @@ sub mkvcbuild
      pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c pqsignal.c
      mkdtemp.c qsort.c qsort_arg.c quotes.c system.c
      sprompt.c tar.c thread.c getopt.c getopt_long.c dirent.c
-     win32env.c win32error.c win32setlocale.c);
+     win32env.c win32error.c win32security.c win32setlocale.c);
 
    push(@pgportfiles, 'rint.c') if ($vsVersion < '12.00');