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

Fix description of errnoRet in Seccomp #1277

Merged

Conversation

z63d
Copy link
Contributor

@z63d z63d commented Feb 3, 2025

There is a subtle difference in syntax between Seccomp's defaultErrnoRet and errnoRet, so i will unify the syntax.

If not specified then its default value is `EPERM`.

If not specified its default value is `EPERM`.

Or you could just reference one of them like this...

* **`defaultAction`** *(string, REQUIRED)* - the default action for seccomp. Allowed values are the same as `syscalls[].action`.

@z63d z63d force-pushed the fix/seccomp-errnoret-descripton branch 3 times, most recently from 8617272 to 5cb64f9 Compare February 3, 2025 23:34
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

I would just say "The default is EPERM" in both cases.

Cc @cyphar

@giuseppe
Copy link
Member

giuseppe commented Feb 4, 2025

agreed. "The default is EPERM" should be enough

@kolyshkin
Copy link
Contributor

@z63d PTAL

@z63d
Copy link
Contributor Author

z63d commented Feb 7, 2025

I just felt strange that there was only one difference in such a sentence.

the errno return code to use.
Some actions like SCMP_ACT_ERRNO and SCMP_ACT_TRACE allow to specify the errno code to return.
When the action doesn't support an errno, the runtime MUST print and error and fail.
If not specified then its default value is EPERM.

If there are no problems, close the PR.

@kolyshkin
Copy link
Contributor

@z63d what I (and @giuseppe) meant is, can you please change the commit do just say "The default is EPERM" in both cases?

@z63d z63d changed the title Fix syntax in Seccomp's errnoRet description Fix description of errnoRet in Seccomp Feb 7, 2025
@z63d z63d force-pushed the fix/seccomp-errnoret-descripton branch from 5cb64f9 to 221c198 Compare February 7, 2025 04:08
@z63d
Copy link
Contributor Author

z63d commented Feb 7, 2025

Ah, my understanding was lacking. Sorry.

@z63d z63d requested a review from kolyshkin February 7, 2025 04:10
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@AkihiroSuda AkihiroSuda added this to the vNext (tentative) milestone Feb 7, 2025
@AkihiroSuda AkihiroSuda merged commit 2f2d37e into opencontainers:main Feb 7, 2025
4 checks passed
@z63d z63d deleted the fix/seccomp-errnoret-descripton branch February 7, 2025 07:23
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.

5 participants