improv: make errors more conclusive #1071
Conversation
kwakubiney
force-pushed
the
improv/make-errors-more-conclusive
branch
from
27a9b8f
to
5ef8223
Compare
kwakubiney
changed the title
link/syscalls.go
Outdated
| @@ -47,10 +47,6 @@ var haveProgAttach = internal.NewFeatureTest("BPF_PROG_ATTACH", "4.10", func() e | |||
| }) | |||
|
|
|||
| var haveProgAttachReplace = internal.NewFeatureTest("BPF_PROG_ATTACH atomic replacement of MULTI progs", "5.5", func() error { | |||
| if err := haveProgAttach(); err != nil { | |||
There was a problem hiding this comment.
Ah, this is an interesting case. Here we have to call haveProgAttach first since it establishes a pre-condition for the feature test to be successful. If I remember correctly it's so that we can reliably get EBADF from the syscall below.
So I think this needs to stay where it is.
There was a problem hiding this comment.
I might have to dig into the why myself.
kwakubiney
force-pushed
the
improv/make-errors-more-conclusive
branch
from
5647c44
to
fe0b65a
Compare
|
There is a real lint error: https://ebpf.semaphoreci.com/jobs/fd7469d0-406a-436d-ad91-19045ab94e1d#L738 |
|
And real test errors: https://ebpf.semaphoreci.com/jobs/84c72b32-bb05-4d61-b48d-c7870fbd4ab4#L649 |
link/syscalls.go
Outdated
| @@ -60,9 +60,11 @@ var haveProgAttachReplace = internal.NewFeatureTest("BPF_PROG_ATTACH atomic repl | |||
| asm.Return(), | |||
| }, | |||
| }) | |||
| if err != nil { | |||
|
|
|||
| if errors.Is(err, unix.EINVAL) { | |||
There was a problem hiding this comment.
You want to keep this change after all?
There was a problem hiding this comment.
Snap, missed it when I was making the changes.
syscalls.go
Outdated
| @@ -87,9 +87,11 @@ var haveMapMutabilityModifiers = internal.NewFeatureTest("read- and write-only m | |||
| MaxEntries: 1, | |||
| MapFlags: unix.BPF_F_RDONLY_PROG, | |||
| }) | |||
|
|
|||
There was a problem hiding this comment.
Nit: gratuitous whitespace change. It's fine to change whitespace to your preference when you're editing a function anyways, but this just increases the diff.
syscalls.go
Outdated
| if err != nil { | ||
| return internal.ErrNotSupported | ||
| if errors.Is(err, unix.EINVAL) { |
There was a problem hiding this comment.
Ditto, do you want to keep EINVAL here after all?
kwakubiney
force-pushed
the
improv/make-errors-more-conclusive
branch
2 times, most recently
from
d8d8ece
to
db050b2
Compare
Signed-off-by: kwakubiney <kebiney@hotmail.com>
kwakubiney
force-pushed
the
improv/make-errors-more-conclusive
branch
from
db050b2
to
5c7f000
Compare
Fixes #891