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=email
 :
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=email
 :
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=email
 :
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=email
 :
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.



--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40632?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I721de21a68a4e336ae508a995e3cfcca05d57efe
Gerrit-Change-Number: 40632
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Mon, 14 Jul 2025 09:01:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <[email protected]>

Reply via email to