Attention is currently required from: pespin.
fixeria 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:
(3 comments)
File libosmo-trx/src/trx_ep.c:
https://gerrit.osmocom.org/c/osmo-trx/+/43112/comment/4fbd68ea_20b15c1c?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.
Done
https://gerrit.osmocom.org/c/osmo-trx/+/43112/comment/de008672_a5377814?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...
Because the caller may call `osmo_trx_ep_free()` right after `osmo_trx_ep_close()`. I could do it the other way around: postpone free()ing the whole `struct osmo_trx_ep` until all of its CTRL iofds are done flushing - does this sound like a better approach?
https://gerrit.osmocom.org/c/osmo-trx/+/43112/comment/12d36029_16b6cf49?usp=... : PS6, Line 581: return !llist_empty(&ep->closing_flushes);
I really wonder why do you need this list of closing flushes. […]
The list exists to let `osmo_trx_ep_free()` find outstanding flushes and sever their back-pointer, so that in `trx_ep_ctrl_flush_write_cb()` we don't access memory that was free()d.
P.S. I am really tired of re-working this patch over and over, and I really wish flushing was implemented in libosmocore. Having to implement this outside of libosmocore feels like a kludge.