pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Dave Page <dpage@postgresql.org>
Cc: pgsql-hackers@postgreSQL.org
Subject: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
Date: Sun, 19 Mar 2017 23:26:40 +0200
Message-ID: <c4350683-434f-750f-1eea-73f58e3450e8@iki.fi> (raw)
In-Reply-To: <16609.1489773427@sss.pgh.pa.us>
References: <E1coo2A-0004wP-SM@gemulon.postgresql.org>
	<16609.1489773427@sss.pgh.pa.us>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>

On 03/17/2017 07:57 PM, Tom Lane wrote:
> Heikki Linnakangas <heikki.linnakangas@iki.fi> writes:
>> Fix and simplify check for whether we're running as Windows service.
>
> This seems to have broken narwhal:
>
> Creating library file: libpostgres.a
> ../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x109): In function `pgwin32_is_admin':
> C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:77: undefined reference to `CheckTokenMembership'
> ../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x127):C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:77: undefined reference to `CheckTokenMembership'
> ../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x264): In function `pgwin32_is_service':
> C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:138: undefined reference to `CheckTokenMembership'
> ../../src/port/libpgport_srv.a(win32security_srv.o)(.text+0x302):C:/msys/1.0/local/pgbuildfarm/buildroot/HEAD/pgsql.build/src/port/win32security.c:163: undefined reference to `CheckTokenMembership'
> collect2: ld returned 1 exit status
> make[2]: *** [postgres] Error 1

Hmm. The other MinGW animals are not having that problem.

According to the MSDN docs [1], CheckTokenMembership() is defined in the 
Advapi32 library. In pg_ctl.c and in 
src/include/common/restricted_token.c, we currently do this, for another 
function that's in Advapi32:

> /*
>  * Mingw headers are incomplete, and so are the libraries. So we have to load
>  * a whole lot of API functions dynamically. Since we have to do this anyway,
>  * also load the couple of functions that *do* exist in minwg headers but not
>  * on NT4. That way, we don't break on NT4.
>  */
> typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
> typedef BOOL (WINAPI * __IsProcessInJob) (HANDLE, HANDLE, PBOOL);
> typedef HANDLE (WINAPI * __CreateJobObject) (LPSECURITY_ATTRIBUTES, LPCTSTR);
> typedef BOOL (WINAPI * __SetInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD);
> typedef BOOL (WINAPI * __AssignProcessToJobObject) (HANDLE, HANDLE);
> typedef BOOL (WINAPI * __QueryInformationJobObject) (HANDLE, JOBOBJECTINFOCLASS, LPVOID, DWORD, LPDWORD);

CheckTokenMembership() is probably one of those missing functions, like 
CreateRestrictedToken.

I'd like to bump our minimum requirements,and stop supporting old MinGW 
versions that don't have those headers. I'm not sure what version they 
were added in, but frogmouth doesn't have this problem, and it uses gcc 
4.5.0. Once we do that, we can simplify pg_ctl.c and restricted_token.c 
to not open advapi32.dll dynamically, and call those functions the usual 
way.

It's not very nice to change the requirements in a minor version, but I 
don't think this would be a real problem for anyone. Not many people 
build PostgreSQL using MinGW, let alone with an ancient version of it. 
But if people don't agree, we could instead revert this patch and apply 
the smaller V2 patch [2] instead, in the back-branches.

Thoughts? Any objections to requiring a newer version of MinGW? Any 
objections to do so in the next minor release?

[1] 
https://msdn.microsoft.com/en-us/library/windows/desktop/aa376389%28v=vs.85%29.aspx

[2] 
https://www.postgresql.org/message-id/CAB7nPqSvfu%3DKpJ%3DNX%2BYAHmgAmQdzA7N5h31BjzXeMgczhGCC%2BQ%40...

- Heikki



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



view thread (14+ messages)  latest in thread

Message-ID: <c4350683-434f-750f-1eea-73f58e3450e8@iki.fi>
Permalink:  ../c4350683-434f-750f-1eea-73f58e3450e8@iki.fi/
Also on:    postgresql.org/message-id/c4350683-434f-750f-1eea-73f58e3450e8@iki.fi

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: hlinnaka@iki.fi, tgl@sss.pgh.pa.us, dpage@postgresql.org, pgsql-hackers@postgreSQL.org
  Subject: Re: [COMMITTERS] pgsql: Fix and simplify check for whether we're running as Windows serv
  In-Reply-To: <c4350683-434f-750f-1eea-73f58e3450e8@iki.fi>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox