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.

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

Program GetFd() return type #51

Closed
geyslan opened this issue Aug 14, 2021 · 5 comments · Fixed by #52
Closed

Program GetFd() return type #51

geyslan opened this issue Aug 14, 2021 · 5 comments · Fixed by #52

Comments

@geyslan
Copy link
Member

geyslan commented Aug 14, 2021

Should we return a Go int instead of C.int?

libbpfgo/libbpfgo.go

Lines 650 to 652 in 871dc67

func (p *BPFProg) GetFd() C.int {
return C.bpf_program__fd(p.prog)
}

Like this:

libbpfgo/libbpfgo.go

Lines 466 to 468 in 871dc67

func (b *BPFMap) GetFd() int {
return int(b.fd)
}

@rafaeldtinoco
Copy link
Contributor

I think so, yes (at least for conformity). And tracee will have to fix some usages:

image

@geyslan
Copy link
Member Author

geyslan commented Aug 15, 2021

If you agree I can raise a PR for both.

@yanivagman
Copy link
Collaborator

Agree. Let's change this to return an int like BPFMap.
@rafaeldtinoco I'm not sure we need to change something in Tracee.
Anyways, let's make this change first and then see if there are any required changes.

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Aug 15, 2021

Sure, let's consider libbpfgo and tracee independent.

Currently I'm checking libbpfgo changes for tracee (specially GetMap, UpdateMap) and I can see that the new implementations of:

  • bpfMap.GetValue()
  • bpfMap.DeleteKey()
  • bpfMap.Update()

are the only functions having unsafe.Pointer() as arguments: Users have to specify C types as arguments instead of relying in old interface{} arguments (that had issues). All the rest of functions seem to have go types as arguments. All of them return go types.

Specially for those 3: bpfMap.GetValue() correctly returns go type: ([]byte, error)

We may need to review return types, so all functions return Go types. @geyslan in your PR, could you re-check if all functions are returning go types (not only GetFD) ? Thanks!

@geyslan
Copy link
Member Author

geyslan commented Aug 16, 2021

I've detected BPFProg.GetFd() as the only one returning a non go type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants