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

BUG: seccomp_rule_add.3: add -EACCES return value #339

Closed
wants to merge 1 commit into from

Conversation

kolyshkin
Copy link
Contributor

The -EACCES return value from seccomp_rule_add and friends was added
by commit 83989be (included into 2.5.0), which tells that this is "part of our
... API promise", so it needs to be documented accordingly. Add it.

The discussion leading to this PR is in seccomp/libseccomp-golang#74,
however we did not came to the agreement whether -EACCES is the best choice.

Fixes: 83989be

@coveralls
Copy link

coveralls commented Sep 24, 2021

Coverage Status

Coverage remained the same at 89.53% when pulling fba2a66 on kolyshkin:eacces-doc into 2457dec on seccomp:main.

@pcmoore pcmoore changed the title doc: seccomp_rule_add.3: add -EACCES return value BUG: seccomp_rule_add.3: add -EACCES return value Sep 28, 2021
@pcmoore pcmoore added this to the v2.5.3 milestone Sep 28, 2021
@pcmoore
Copy link
Member

pcmoore commented Sep 28, 2021

Thanks @kolyshkin.

I'm still wondering if we are better off returning -EACCES in this case or changing it to -EEXIST, but I guess sticking with -EACCES is probably for the best as that is what we have done starting with v2.5.x and the beginning of our "API promise".

My next question is if we want to be a specific as this patch when explaining the reason for the error code, "The action argument equals the default action of the filter" is pretty specific. Perhaps consider changing it to: "The rule conflicts with the default filter settings"?

Thoughts?

@kolyshkin
Copy link
Contributor Author

My next question is if we want to be a specific as this patch when explaining the reason for the error code, "The action argument equals the default action of the filter" is pretty specific. Perhaps consider changing it to: "The rule conflicts with the default filter settings"?

Thoughts?

What about something like "The rule conflicts with the filter (for example, the rule action equals the default action of the filter)"? This way we are specific (and clear) but still leave room for other similar errors.

@drakenclimber
Copy link
Member

My next question is if we want to be a specific as this patch when explaining the reason for the error code, "The action argument equals the default action of the filter" is pretty specific. Perhaps consider changing it to: "The rule conflicts with the default filter settings"?
Thoughts?

What about something like "The rule conflicts with the filter (for example, the rule action equals the default action of the filter)"? This way we are specific (and clear) but still leave room for other similar errors.

I'm fine with either of these.

The -EACCES return value from seccomp_rule_add* was added by commit
83989be (included into 2.5.0), which tells that this is "part of our
... API promise", so it needs to be documented accordingly. Add it.

Fixes: 83989be
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

Updated to

       -EACCCES
              The rule conflicts with the filter (for example, the rule action
              equals the default action of the filter).

and fix a typesetting error I made earlier (missing .TP).

@pcmoore
Copy link
Member

pcmoore commented Oct 8, 2021

Since @drakenclimber is okay with either approach I went ahead and merged this via 50da6c1, thanks @kolyshkin!

@pcmoore pcmoore closed this Oct 8, 2021
@pcmoore
Copy link
Member

pcmoore commented Oct 8, 2021

Ported to the release-2.5 branch via 5535f14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants