-
Notifications
You must be signed in to change notification settings - Fork 16
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
disable comparison linter for errors from golang.org/x/sys/unix #10
Comments
https://pkg.go.dev/golang.org/x/sys/unix#section-documentation says:
Summoning @tklauser to confirm (or deny) that there are no plans to change that, i.e. the errors returned from unix pkg will always be bare errnos. |
That's correct. As far as I know there are no plans to change that. FWIW, the tests in the |
One thing that does concern me about the current approach is that it uses only the package path basename which can become ambiguous with multiple packages. If the scope of the allowlist will be greater than just the standard library it is a good idea to match on absolute paths. |
Based on golang/go#40827, it seems like only io.EOF is really something which should not be wrapped while others might be? |
@mitar Yes, you are right. I had taken the current approach because there is not really anything preventing people from still wrapping io.EOF in their own code. Ignoring io.EOF altogether should be complemented by a lint that detects wrapping io.EOF |
This is my naive (and incomplete) approach to fix GH issue polyfloyd#10. The problems with this implementation are: - no test cases (it seems those would need the whole golang.org/x/sys/unix to be available under testdata/src); - a check is too relaxed (ignoring all errors that start from "E" from any package named "unix"). Alas, I have no idea how to fix those. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is my naive (and incomplete) approach to fix GH issue polyfloyd#10. The problems with this implementation are: - no test cases (it seems those would need the whole golang.org/x/sys/unix to be available under testdata/src); - a check is too relaxed (ignoring all errors that start from "E" from any package named "unix"). Alas, I have no idea how to fix those. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
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 Co-Authored-by: polyfloyd <floyd@polyfloyd.net> Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Functions from the
golang.org/x/sys/unix
package wraps its error, always returning bare errno, and so it's OK to use direct error comparison.Example:
On the above code, the linter says:
For a package that uses a lot of
unix
functions, it's a burden to annotate all such code as the above example with//nolint:errorlint // unix errors are bare
, but this is exactly what I had to do in runc: opencontainers/runc@56e4780Surely, the alternative is to switch to
errors.Is(err, unix.ENOENT)
and the like, but in the particular case it seems redudnant.Would be nice to have comparison linter to add an exclusion for errors returned from golang.org/x/sys/unix.* functions.
The text was updated successfully, but these errors were encountered: