Thread: Setting restrictedtoken in pg_regress
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. -- Daniel Gustafsson https://vmware.com/
Attachment
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
Attachment
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 aboolean test like the above. Done that way and pushed, thanks! -- Daniel Gustafsson