-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Use errors.Unwrap() where possible #2280
Conversation
This simplifies code a lot. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Tested that the EINTR is still being detected: > $ go1.14 test -c # 1.14 is needed for EINTR to happen > $ sudo ./fscommon.test > INFO[0000] interrupted while writing 1063068 to /sys/fs/cgroup/memory/test-eint-89293785/memory.limit_in_bytes > PASS Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Let me know if you want me to merge most of the commits (or all of them) |
1 similar comment
Oh well, this might not be working as I thought it would. See this example: https://play.golang.org/p/-HyjwI4v_pz IOW if an error is not wrapped, |
We should revert this PR? |
errno = err.Err | ||
} | ||
// Handle some specific syscall errors. | ||
errno := errors.Unwrap(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code might not be working the way it should. If err
is not wrapped, errno will be nil
.
Will do a followup PR.
…unwrap" Using errors.Unwrap() is not the best thing to do, since it returns nil in case of an error which was not wrapped. More to say, errors package provides more elegant ways to check for underlying errors, such as errors.As() and errors.Is(). This reverts commit f8e1388, reversing changes made to 6ca9d8e. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@AkihiroSuda while I don't see any code paths that might hit the problem I described above, it's better to not use |
This set makes use of errors.Unwrap() where possible to get to an underlying error. The biggest motivation is to simplify the code.
The feature requires go 1.13 but since merging #2256 we are already not supporting go 1.12 (which is an unsupported release anyway).