Skip to content

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 <maksiman@microsoft.com>
@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
@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 <maksiman@microsoft.com>
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 <maksiman@microsoft.com>
(cherry picked from commit 42671b4)
Signed-off-by: Kirtana Ashok <kiashok@microsoft.com>
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 <maksiman@microsoft.com>
(cherry picked from commit 42671b4)
Signed-off-by: Kirtana Ashok <kiashok@microsoft.com>
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