Attention is currently required from: pespin.

View Change

3 comments:

• File libosmo-trx/src/trx_ep.c:

  • Patch Set #6, 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

  • Patch Set #6, 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?

  • Patch Set #6, 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.

To view, visit change 43112. To unsubscribe, or for help writing mail filters, visit settings.