Setting restrictedtoken in pg_regress

On Tue, Aug 30, 2022 at 03:02:54PM +0200, Daniel Gustafsson wrote:

In pg_regress we set restrictedToken when calling CreateRestrictedProcess, but
we never seem to use that anywhere. Not being well versed in Windows I might
be missing something, but is it needed or is it a copy/pasteo from fa1e5afa8a2
which does that in restricted_token.c? If not needed, removing it makes the
code more readable IMO.

Looks reasonable to me.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

On Mon, Jun 12, 2023 at 04:12:22PM -0700, Nathan Bossart wrote:

On Tue, Aug 30, 2022 at 03:02:54PM +0200, Daniel Gustafsson wrote:

In pg_regress we set restrictedToken when calling CreateRestrictedProcess, but
we never seem to use that anywhere. Not being well versed in Windows I might
be missing something, but is it needed or is it a copy/pasteo from fa1e5afa8a2
which does that in restricted_token.c? If not needed, removing it makes the
code more readable IMO.

Looks reasonable to me.

Indeed, looks like a copy-pasto to me.

I am actually a bit confused with the return value of
CreateRestrictedProcess() on failures in restricted_token.c. Wouldn't
it be cleaner to return INVALID_HANDLE_VALUE rather than 0 in these
cases?
--
Michael

On Tue, Jun 13, 2023 at 08:29:19AM +0900, Michael Paquier wrote:

I am actually a bit confused with the return value of
CreateRestrictedProcess() on failures in restricted_token.c. Wouldn't
it be cleaner to return INVALID_HANDLE_VALUE rather than 0 in these
cases?

My suspicion is that this was chosen to align with CreateProcess and to
allow things like

if (!CreateRestrictedProcess(...))

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

On 2023-06-12 Mo 19:43, Nathan Bossart wrote:

On Tue, Jun 13, 2023 at 08:29:19AM +0900, Michael Paquier wrote:

I am actually a bit confused with the return value of
CreateRestrictedProcess() on failures in restricted_token.c. Wouldn't
it be cleaner to return INVALID_HANDLE_VALUE rather than 0 in these
cases?

My suspicion is that this was chosen to align with CreateProcess and to
allow things like

if (!CreateRestrictedProcess(...))

Probably, it's been a while. I doubt it's worth changing at this point,
and we could just change pg_regress.c to use a boolean test like the above.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

On 14 Jun 2023, at 13:02, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2023-06-12 Mo 19:43, Nathan Bossart wrote:

On Tue, Jun 13, 2023 at 08:29:19AM +0900, Michael Paquier wrote:

I am actually a bit confused with the return value of
CreateRestrictedProcess() on failures in restricted_token.c. Wouldn't
it be cleaner to return INVALID_HANDLE_VALUE rather than 0 in these
cases?

My suspicion is that this was chosen to align with CreateProcess and to
allow things like

if (!CreateRestrictedProcess(...))

Probably, it's been a while. I doubt it's worth changing at this point, and we could just change pg_regress.c to use a boolean test like the above.

Done that way and pushed, thanks!

--
Daniel Gustafsson