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.

Sign up for GitHub

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

Jump to bottom

Allow bare errors from golang.org/x/sys/unix #47

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Allow bare errors from golang.org/x/sys/unix #47

merged 1 commit into from
Aug 2, 2023

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Jul 27, 2023 •
edited
Loading

All functions in golang.org/x/sys/unix always return raw errno values,
which can be compared directly.

There are about 100-130 errors, and about 350 functions in unix package.
Listing them separately is hardly an option, so let's ignore all unix.E*
errors that come from all functions in unix package.

This functionality is made possible thanks to commit cd16050.

Add a small test case (we can't really add the whole x/sys/unix under
testdata, so use a stub implementation).

Fixes: #10

@kolyshkin

This comment was marked as outdated.

Sign in to view
@polyfloyd
Copy link
Owner

Thanks for reminding me of this. I have since #10 written another linter that is able to get the full module path. I'll see if I can replicate that here. If that turns out to be not possible, then I propose we merge your proposal. There will probably not a lot of Go packages named unix :)

@polyfloyd
Copy link
Owner

Ah! I did it. Allowed errors and functions can now be checked against the full package path. I have pushed some unix error handling to the ignore-unix-err branch. Hope this helps!

@kolyshkin @polyfloyd
fix: ignore unix errno values
6464b7e 
All functions in golang.org/x/sys/unix always return raw errno values,
which can be compared directly.

There are about 100-130 errors, and about 350 functions in unix package.
Listing them separately is hardly an option, so let's ignore all unix.E*
errors that come from all functions in unix package.

This functionality is made possible thanks to commit cd16050.

Add a small test case (we can't really add the whole x/sys/unix under
testdata, so use a stub implementation).

Fixes: polyfloyd#10

Co-Authored-by: polyfloyd <floyd@polyfloyd.net>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

Awesome, I've updated this PR based on your code, PTAL.

Since there are 100-130 errors in unix (depending on the GOOS and GOARCH), and about 350 functions that can return those errors, ignoring individual errors from the individual functions is hardly an option. We need a wildcard here.

@kolyshkin
Copy link
Contributor Author

Thanks a lot!

Tested this PR on github.com/opencontainers/runc, no errors from unix are reported (aside from a single unrelated issue, reported in #49.

@polyfloyd polyfloyd merged commit 41edee2 into polyfloyd:main Aug 2, 2023
2 checks passed
@polyfloyd
Copy link
Owner

Great, thanks. Good to have this finally fixed :)

This was referenced Aug 15, 2023
Merged
Merged
kolyshkin added a commit to kolyshkin/runc that referenced this pull request Aug 25, 2023
@kolyshkin
Remove nolint annotations for unix errno comparisons
f62f0bd 
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix.

Thus, these annotations are no longer necessary. Hooray!

[1] polyfloyd/go-errorlint#47
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This was referenced Aug 25, 2023
Merged
Merged
kolyshkin added a commit to kolyshkin/sys that referenced this pull request Aug 25, 2023
@kolyshkin
mount,mountinfo: rm unneeded errorlint annotations
344c06b 
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix.

Thus, these annotations are no longer necessary. Hooray!

[1] polyfloyd/go-errorlint#47

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/sys that referenced this pull request Aug 25, 2023
@kolyshkin
mount: rm unneeded errorlint annotation
a317ea2 
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix. Remove the annotation that is no longer needed.

Unfortunately, switch on a bare unix error (in mountinfo) still needs to
be annotated (see [2]).

[1] polyfloyd/go-errorlint#47
[2] polyfloyd/go-errorlint#54

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/selinux that referenced this pull request Aug 25, 2023
@kolyshkin
Remove nolint annotations for unix errno comparisons
5ab22c5 
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix. Thus, these annotations are no longer necessary. Remove
those.

Except, the errors that do not come directly from a function in unix
package still need to be annotated (see [2]).

[1] polyfloyd/go-errorlint#47
[2] polyfloyd/go-errorlint#55
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin mentioned this pull request Aug 25, 2023
Merged
kolyshkin added a commit to kolyshkin/sys that referenced this pull request Aug 25, 2023
@kolyshkin
mount: rm unneeded errorlint annotation
1364dae 
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix. Remove the annotation that is no longer needed.

Unfortunately, switch on a bare unix error (in mountinfo) still needs to
be annotated (see [2]).

[1] polyfloyd/go-errorlint#47
[2] polyfloyd/go-errorlint#54

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/sys that referenced this pull request Aug 25, 2023
@kolyshkin
mount: rm unneeded errorlint annotation
b474724 
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix. Remove the annotation that is no longer needed.

Unfortunately, switch on a bare unix error (in mountinfo) still needs to
be annotated (see [2]).

[1] polyfloyd/go-errorlint#47
[2] polyfloyd/go-errorlint#54

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/sys that referenced this pull request Oct 27, 2023
@kolyshkin
mount: rm unneeded errorlint annotation
d6a5209 
golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains
the fix [1] whitelisting all errno comparisons for errors coming from
x/sys/unix. Remove the annotation that is no longer needed.

Unfortunately, switch on a bare unix error (in mountinfo) still needs to
be annotated (see [2]).

[1] polyfloyd/go-errorlint#47
[2] polyfloyd/go-errorlint#54

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

disable comparison linter for errors from golang.org/x/sys/unix
2 participants
@kolyshkin @polyfloyd