Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update JOB_OBJECT_ALL_ACCESS and OpenJobObject #2095

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

anmaxvl
Copy link
Contributor

@anmaxvl anmaxvl commented Apr 1, 2024

Update JOB_OBJECT_ALL_ACCESS value to the most recent one.
Update winapi.OpenJobObject to accept inheritHandle as bool
as the documentation suggests.

Update `JOB_OBJECT_ALL_ACCESS` value to the most recent one.
Update `winapi.OpenJobObject` to accept `inheritHandle` as
`bool` as the documentation suggests.

Signed-off-by: Maksim An <[email protected]>
@anmaxvl anmaxvl requested a review from a team as a code owner April 1, 2024 19:38
@kevpar
Copy link
Member

kevpar commented Apr 1, 2024

What is the documentation that suggests we change to passing bool directly to the generated function?

@anmaxvl
Copy link
Contributor Author

anmaxvl commented Apr 1, 2024

What is the documentation that suggests we change to passing bool directly to the generated function?

I wanted to make sure that the type matches to what's in the docs: https://learn.microsoft.com/en-us/windows/win32/api/jobapi2/nf-jobapi2-openjobobjectw. The underlying syscall doesn't change, and with the change we won't need to convert bools to uint32 in the future, if we ever want to add inheritHandle to https://github.com/microsoft/hcsshim/blob/main/internal/jobobject/jobobject.go#L63

@kevpar
Copy link
Member

kevpar commented Apr 1, 2024

It looks like the syscall code handles this just fine. Just wanted to be clear that in general, Windows BOOL and Go's bool are not compatible types.

Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@anmaxvl anmaxvl merged commit 42671b4 into microsoft:main Apr 2, 2024
19 checks passed
@anmaxvl anmaxvl deleted the jobobject-updates branch April 2, 2024 17:38
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
Update `JOB_OBJECT_ALL_ACCESS` value to the most recent one.
Update `winapi.OpenJobObject` to accept `inheritHandle` as
`bool`. The underlying syscall stays the same, but this allows
cleaner calls from go's perspective as it avoids `bool` to `uint32`
casting.

Signed-off-by: Maksim An <[email protected]>
kiashok pushed a commit to kiashok/hcsshim that referenced this pull request Oct 29, 2024
Update `JOB_OBJECT_ALL_ACCESS` value to the most recent one.
Update `winapi.OpenJobObject` to accept `inheritHandle` as
`bool`. The underlying syscall stays the same, but this allows
cleaner calls from go's perspective as it avoids `bool` to `uint32`
casting.

Signed-off-by: Maksim An <[email protected]>
(cherry picked from commit 42671b4)
Signed-off-by: Kirtana Ashok <[email protected]>
kiashok pushed a commit that referenced this pull request Oct 30, 2024
Update `JOB_OBJECT_ALL_ACCESS` value to the most recent one.
Update `winapi.OpenJobObject` to accept `inheritHandle` as
`bool`. The underlying syscall stays the same, but this allows
cleaner calls from go's perspective as it avoids `bool` to `uint32`
casting.

Signed-off-by: Maksim An <[email protected]>
(cherry picked from commit 42671b4)
Signed-off-by: Kirtana Ashok <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants