Fix some more error handling #163
Conversation
This PR doesn't address all the cases, including, but not limited to the batch APIs
| return fmt.Errorf("failed to resize map %s to %v: %w", b.name, maxEntries, syscall.Errno(-errC)) | ||
| ret, errC := C.bpf_map__set_max_entries(b.bpfMap, C.uint(maxEntries)) | ||
| if ret != 0 { | ||
| return fmt.Errorf("failed to resize map %s to %v: %w", b.name, maxEntries, errC) |
There was a problem hiding this comment.
Why don't we want to use syscall.Errno(-errC)? In these cases you're using %w for errC, which is an int.
There was a problem hiding this comment.
The second argument's type of any CGO call is error, which is the errno already.
It's confusing because Go doesn't require us to bind all the return values of a function, and before, as errC was the sole function return value we were reading, it was in reality the return value, which is a C.int, and not what we want.
That's why passing it to syscall.Errno always resulted in -1, EPERM(#147) [1]
[1]: For completeness, it returns -1on failure before libbpf 1.0, which hasn't been released yet, or if LibbpfStrictModeDirectErrs is not used (https://github.com/libbpf/libbpf/wiki/Libbpf-1.0-migration-guide).
Hope this makes sense, let me know if this is not what you meant!
There was a problem hiding this comment.
Ah yes of course, my confusion. I'd like to standardize naming of CGO errors that are int's as errorCode and the second return of type error as errno. I can handle that in the future.
| return fmt.Errorf("failed to resize map %s to %v: %w", b.name, maxEntries, syscall.Errno(-errC)) | ||
| ret, errC := C.bpf_map__set_max_entries(b.bpfMap, C.uint(maxEntries)) | ||
| if ret != 0 { | ||
| return fmt.Errorf("failed to resize map %s to %v: %w", b.name, maxEntries, errC) |
There was a problem hiding this comment.
Ah yes of course, my confusion. I'd like to standardize naming of CGO errors that are int's as errorCode and the second return of type error as errno. I can handle that in the future.
This PR doesn't address all the call-sites that need to have their return error checked but focuses on some APIs that I am experimenting with. For more context, see: #159
(this PR does not tackle the remaining batch API calls #162)
Thanks!