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

Add strict mode API #160

Merged
merged 1 commit into from May 3, 2022

Conversation

javierhonduco
Copy link
Contributor

@javierhonduco javierhonduco commented Apr 28, 2022

@javierhonduco javierhonduco force-pushed the add-strict-mode-api branch 2 times, most recently from 222261b to 96eb304 Compare Apr 28, 2022
@grantseltzer grantseltzer self-requested a review Apr 28, 2022
Copy link
Member

@grantseltzer grantseltzer left a comment

Just the one comment, this is a very welcome addition, thank you very much!

libbpfgo.go Outdated Show resolved Hide resolved
@javierhonduco
Copy link
Contributor Author

javierhonduco commented Apr 28, 2022

Addressed review feedback + added some more strict mode values

libbpfgo.go Show resolved Hide resolved
libbpfgo.go Show resolved Hide resolved
libbpfgo.go Outdated Show resolved Hide resolved
libbpfgo.go Show resolved Hide resolved
@javierhonduco
Copy link
Contributor Author

javierhonduco commented Apr 28, 2022

Addressed feedback

}

func SetStrictMode(mode LibbpfStrictMode) {
C.libbpf_set_strict_mode(uint32(mode))
Copy link
Member

@grantseltzer grantseltzer Apr 28, 2022

Choose a reason for hiding this comment

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

Suggested change
C.libbpf_set_strict_mode(uint32(mode))
C.libbpf_set_strict_mode(C.uint(mode))

Copy link
Member

@grantseltzer grantseltzer Apr 28, 2022

Choose a reason for hiding this comment

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

Last thing, then good to merge 👍

Copy link
Contributor Author

@javierhonduco javierhonduco May 3, 2022

Choose a reason for hiding this comment

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

Adding this errors with:

../libbpfgo/libbpfgo.go:365:27: cannot use _Ctype_uint(mode) (value of type _Ctype_uint) as type uint32 in argument to (_Cfunc_libbpf_set_strict_mode)

Seems like uint32 is the right type 😄. Passing a "Go" type seems recurrent for enums. I tried searching for this behaviour, but my Google-fu failed me

https://github.com/aquasecurity/libbpfgo/blob/main/libbpfgo.go#L1158-L1160

Copy link
Member

@grantseltzer grantseltzer May 3, 2022

Choose a reason for hiding this comment

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

That's so strange, I can in fact reproduce that.

I did some googling and apparently enums are simplified to uint32's in CGO wrappers. This issue isn't the same as ours specifically, but it does mention this. Learn something new everyday :-)

Copy link
Contributor Author

@javierhonduco javierhonduco May 3, 2022

Choose a reason for hiding this comment

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

TIL too!! :)

Copy link
Member

@grantseltzer grantseltzer left a comment

L G T M !

@grantseltzer grantseltzer merged commit 80f4430 into aquasecurity:main May 3, 2022
1 check passed
@javierhonduco javierhonduco deleted the add-strict-mode-api branch May 3, 2022
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 this pull request may close these issues.

None yet

2 participants