rust-bpf / rust-bcc

user-friendly rust bindings for the bpf compiler collection
MIT License
475 stars 54 forks source link

add BPFBuilder with cflags support #128

Closed brayniac closed 4 years ago

brayniac commented 4 years ago

Adds a builder pattern for instantiating a new BPF module with support for cflags.

Provides functionality to match intent of #127 with a more idiomatic pattern for construction.

Non-breaking change, users can migrate to the new pattern if required. We may consider deprecating the original constructor eventually, but for now it is left as-is.

javierhonduco commented 4 years ago

Perhaps this is not idiomatic Rust but should we store what we mem::forget in a "private" field of our struct so we can drop it later?

Right now we are leaking that memory, which is not a huuuge deal for small CLIs but in some long running apps that create many BPFBuilder this will eventually add up. On top of that, the memory/leak sanitizers' output won't be clean anymore and we might lose some signal.

What do you think?

brayniac commented 4 years ago

Perhaps this is not idiomatic Rust but should we store what we mem::forget in a "private" field of our struct so we can drop it later?

Right now we are leaking that memory, which is not a huuuge deal for small CLIs but in some long running apps that create many BPFBuilder this will eventually add up. On top of that, the memory/leak sanitizers' output won't be clean anymore and we might lose some signal.

What do you think?

You're correct - this should be fixed before releasing.