Yeah I am having the landscape of choices mapped out, and I'd like to get a
feel for the general opinion to fine tune my heading.
"At what degree of which aspects do you guys see the optimum?"
On Mon, Feb 26, 2024 at 10:57:16AM +0100, Pau Espin Pedrol wrote:
As usual, I'd go for opaque. Since opaque means
the implementation can be
changed, then it doesn't matter much whether you want to use dynamic or
static. It can be a mix of both, use whatever fits more in each place.
An opaque API has to be dynamic.
There is always a struct foo * that was returned by a dynamic foo_alloc().
But I'd definitely make the lib allocate the
structs and have them as opaque
pointers.
Ok, poll taken: the benefit of easily modifying the API later weighs more than
having to write dozens of foo_get_x() and foo_set_x() and calling functions all
the time instead of using plain pointer arithmetic.
Regarding "benefits of iterating over
llist", you can simply have an API
return the llist pointer and have some API to get the object from the llist
per-item pointer which is internally implemented by means of container_of()
or a cast or similar.
Yes. But is it really opaque to return an llist_head? Should the API maybe hide
the particular list implementation and provide functions for walking the list?
What I'd also like your opinion on is, how far would you take the dynamic-ness?
Is it fine to have many detailed allocation "stages", for the benefit of logic
and opacity? Example:
I like it when the object itself is the talloc context for its members:
struct codec_list *my_list = codec_list_alloc(ctx);
codec_list_add(my_list, item); // <-- use my_list as talloc parent for the new
entry
That means I want to allocate an llist_head instance dynamically. Going this
way makes a lot more, small dynamic allocations.
(benefit: the logical structure of the data shows very nicely in the talloc
reports!)
An alternative would be: use an llist_head that the caller simply has as static
member of its state:
struct llist_head my_list;
codec_list_add(&my_list, ctx, item); // <-- always provide the talloc ctx
separately
It has less separate dynamic allocations, but there are more ways to shoot
oneself in the foot, and more ctx arguments all over the API.
Also it's not really opaque anymore.
At the moment I want to go for many small dynamic allocations.
Fine?
Ah and also, when the osmo_sdp_codec{} is opaque, how would we set up the data
structures in codec_mapping.c? I guess with a function that procedurally builds
an allocated list? Currently it's an (IMHO nice) const array of pure data:
https://gerrit.osmocom.org/plugins/gitiles/osmo-msc/+/master/src/libmsc/cod…
If struct osmo_sdp_codec is transparent, it can stay this way. If I make it
opaque, every part here that says .sdp = {...} would have to be dynamically
created via API instead.
(In my current status, osmo_sdp_codec is transparent. If the opinions are
strong I'll add effort to make it opaque)
Another idea would be to invent a separate SDP struct in osmo-msc, but that
also needs more API to convert one to the other... I don't want that.
Thanks!!
~N