Attention is currently required from: neels, pespin.

View Change

9 comments:

• Patchset:

  • Patch Set #5:

    Thanks for the detailed review, updated.

• File include/osmocom/bsc_nat/bsc_nat_fsm.h:

  • Patch Set #4, Line 21: enum bsc_nat_fsm_states {

    (usually the state enum can live in the foo_fsm. […]

    Done

  • Patch Set #4, Line 33: extern struct osmo_fsm bsc_nat_fsm;

    (no need to declare the fsm in the .h, it can be static in the .c file. […]

    Done

  • Patch Set #4, Line 35: void bsc_nat_start(struct bsc_nat *bsc_nat);

    does this do the same as BSC_NAT_FSM_EV_START? Then maybe the events enum can also move to the . […]

    Done

• File src/osmo-bsc-nat/bsc_nat_fsm.c:

  • Patch Set #4, Line 34: #define X(s) (1 << (s))

    There's tons of FSMs where X() is used.

    keeping X()

  • Patch Set #4, Line 142: osmo_fsm_inst_state_chg(fi, BSC_NAT_FSM_ST_STOPPED, 0, 0);

    seems like ST_STOPPING is not needed: the onenter directly changes state to another state. […]

    Done

  • Patch Set #4, Line 159: X(BSC_NAT_FSM_ST_STOPPING),

    (let me plug here the scheme i invented and use in my FSM definitions, which allows adding and remov […]

    Done

  • Patch Set #4, Line 184: int bsc_nat_fsm_timer_cb(struct osmo_fsm_inst *fi)

    seems like no timer cb is needed. […]

    Done

• File src/osmo-bsc-nat/main.c:

  • Patch Set #4, Line 150: bsc_nat_stop(g_bsc_nat, true);

    osmo_select_shutdown_request() changes the osmo select loop to only handle pending writes and no lon […]

    Done

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