Skip to content

Fix description of errnoRet in Seccomp#1277

Merged
AkihiroSuda merged 1 commit intoopencontainers:mainfrom
z63d:fix/seccomp-errnoret-descripton
Feb 7, 2025
Merged

Fix description of errnoRet in Seccomp#1277
AkihiroSuda merged 1 commit intoopencontainers:mainfrom
z63d:fix/seccomp-errnoret-descripton

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.

runtime-spec/config-linux.md

Line 722 in a5b0116

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

runtime-spec/config-linux.md

Line 797 in a5b0116

If not specified its default value is `EPERM`.

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

runtime-spec/config-linux.md

Line 718 in a5b0116

* **`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
kolyshkin
kolyshkin reviewed Feb 3, 2025
View reviewed changes
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

kolyshkin commented Feb 7, 2025

@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

kolyshkin commented Feb 7, 2025

@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
kolyshkin
kolyshkin approved these changes Feb 7, 2025
View reviewed changes
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

Reviewers

@tianon tianon tianon approved these changes

@kolyshkin kolyshkin kolyshkin approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@crosbymichael crosbymichael Awaiting requested review from crosbymichael crosbymichael is a code owner

@cyphar cyphar Awaiting requested review from cyphar cyphar is a code owner

@dqminh dqminh Awaiting requested review from dqminh dqminh is a code owner

@giuseppe giuseppe Awaiting requested review from giuseppe giuseppe is a code owner

@hqhq hqhq Awaiting requested review from hqhq hqhq is a code owner

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner

@thaJeztah thaJeztah Awaiting requested review from thaJeztah thaJeztah is a code owner

@utam0k utam0k Awaiting requested review from utam0k utam0k is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.2.1

Development

Successfully merging this pull request may close these issues.

5 participants

@z63d @giuseppe @kolyshkin @tianon @AkihiroSuda