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

Use libbpfgo/helpers package in tracee-ebpf #631

Merged
merged 2 commits into from
Mar 18, 2021
Merged

Use libbpfgo/helpers package in tracee-ebpf #631

merged 2 commits into from
Mar 18, 2021

Conversation

grantseltzer
Copy link
Contributor

This is a follow up to #493, which should fully close out #485

Question I have is whether or not we want to copy over the PrintUint32IP and Print16BytesSliceIP to this helpers package. Not sure if it's relevant enough?

…elper package

Signed-off-by: grantseltzer <grantseltzer@gmail.com>
helpers package

Signed-off-by: grantseltzer <grantseltzer@gmail.com>
@yanivagman
Copy link
Collaborator

Question I have is whether or not we want to copy over the PrintUint32IP and Print16BytesSliceIP to this helpers package. Not sure if it's relevant enough?

I don't think we should copy these helpers. The string representation of an address can be considered as "raw" by itself

Copy link
Collaborator

@yanivagman yanivagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, but maybe we should remove the argprinters.go file and move the functions left (PrintUint32IP, Print16BytesSliceIP, PrintAlert) to tracee.go?

@grantseltzer
Copy link
Contributor Author

@yanivagman I think 1600 lines is enough for tracee.go, to be honest I wouldn't mind breaking it up further to make it easier to read/navigate.

Lets handle that in a future discussion/PR

@grantseltzer grantseltzer merged commit 1a31966 into aquasecurity:main Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants