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');