Horton

Thread

Commits

  1. pg_regress: Remove unused variable

  1. Setting restrictedtoken in pg_regress

    Daniel Gustafsson <daniel@yesql.se> — 2022-08-30T13:02:54Z

    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/

  2. Re: Setting restrictedtoken in pg_regress

    Nathan Bossart <nathandbossart@gmail.com> — 2023-06-12T23:12:22Z

    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

  3. Re: Setting restrictedtoken in pg_regress

    Michael Paquier <michael@paquier.xyz> — 2023-06-12T23:29:19Z

    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

  4. Re: Setting restrictedtoken in pg_regress

    Nathan Bossart <nathandbossart@gmail.com> — 2023-06-12T23:43:09Z

    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

  5. Re: Setting restrictedtoken in pg_regress

    Andrew Dunstan <andrew@dunslane.net> — 2023-06-14T11:02:30Z

    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

  6. Re: Setting restrictedtoken in pg_regress

    Daniel Gustafsson <daniel@yesql.se> — 2023-07-06T20:10:16Z

    > 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