Attention is currently required from: fixeria, neels.

pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40632?usp=email )

Change subject: ran_peer: Add specific API to free object ......................................................................

Patch Set 2:

(4 comments)

Commit Message:

https://gerrit.osmocom.org/c/osmo-msc/+/40632/comment/35436821_5f040e5e?usp=... : PS2, Line 11: Swap the talloc tree parentship to have the fi be a child of the object, : which simplifies tear down triggered by FSM cleanup.

From the hierarchical point of view, I find it logical the way it is: the FSM instance's private dat […]

It's logical, but it has drawbacks, like not being able to quickly figuring out where the object is freed and hence it's entire lifecycle.

https://gerrit.osmocom.org/c/osmo-msc/+/40632/comment/8e5ef0d8_162ca47d?usp=... : PS2, Line 14: Apparently the object is not being freed anywhere

It's being free()d automatically by talloc, since it's allocated as a child of the fi: […]

I know it's freed by talloc automatically. Now, can you tell me where is the fsm instance being terminated/freed in the codebase? It's never freed anywhere. That's what I meant.

And yes, the fact that it's freed automatically by terminating/freeing the FSM doesn't really help precisely figuring out where this object is freed, because there's no specific API to grep for in the code.

File include/osmocom/msc/ran_peer.h:

https://gerrit.osmocom.org/c/osmo-msc/+/40632/comment/9da0baec_a9c6ed7b?usp=... : PS2, Line 109: ran_peer_discard_all_conns

How is this related to this patch? Why exposing this API?

It's called by ran_peer_free(), so I need it declared beforehand. It was already not set as static before, so I'm not really changing that here.

I can make it static and add a forward declaration in the .c file if you want.

File src/libmsc/ran_peer.c:

https://gerrit.osmocom.org/c/osmo-msc/+/40632/comment/9eeca83e_84219fe3?usp=... : PS2, Line 79: ran_peer_free

You're adding this new API, but it's not being called anywhere. […]

Yeah well, these objects are not actually being freed anywhere in code (yet?). Still, it's good to have a clear lifecycle with all the logic so no bugs are introduced whenever somebody needs freeing them at some point later.