Attention is currently required from: fixeria.

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

Change subject: libosmo-trx/ep: flush pending TRXC messages on endpoint teardown ......................................................................

Patch Set 6: Code-Review+1

(5 comments)

Patchset:

PS6: As mentioned I don't think trying to hide the asynchronous close of ctrl socket by outliving the object is a good idea, and I think that will come up with problems later on. But if you are happy with it, I don't want to block this.

File libosmo-trx/src/trx_ep.c:

https://gerrit.osmocom.org/c/osmo-trx/+/43112/comment/0a46363a_36fd5a71?usp=... : PS6, Line 310: LOGEPCH(flush->ep, flush->chan_num, LOGL_DEBUG, This should be inside the if above, since no flush can complete if res <=0.

https://gerrit.osmocom.org/c/osmo-trx/+/43112/comment/92f193ad_fc428d07?usp=... : PS6, Line 348: flush = talloc(iofd, struct trx_ep_ctrl_flush); I really fail to see why do we need a separate object to handle the flushing, but ok...

https://gerrit.osmocom.org/c/osmo-trx/+/43112/comment/d6d319f0_97e7d2cb?usp=... : PS6, Line 552: * finish flushing on its own, since it may outlive the endpoint. */ I don't really see why would you like the ctrl socket to outlive the endpoint. If the ctrl socket needs more time to close, then closing the endpoint should also be an asynchronous operation, with a closed_cb.

https://gerrit.osmocom.org/c/osmo-trx/+/43112/comment/95453248_97e6d59f?usp=... : PS6, Line 581: return !llist_empty(&ep->closing_flushes); I really wonder why do you need this list of closing flushes. IMHO it's much more simple: if CTRL socket in any chan is_closing(), then osmo_trx_ep is_closing().