Attention is currently required from: neels, pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662 )
Change subject: Add bsc_nat fsm ......................................................................
Patch Set 5:
(9 comments)
Patchset:
PS5: Thanks for the detailed review, updated.
File include/osmocom/bsc_nat/bsc_nat_fsm.h:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662/comment/8b63849b_0bba3e23 PS4, Line 21: enum bsc_nat_fsm_states {
(usually the state enum can live in the foo_fsm. […]
Done
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662/comment/a90c2712_7a6e96fd PS4, 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
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662/comment/f5560898_43a60a2b PS4, 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:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662/comment/47801b12_3e07cc73 PS4, Line 34: #define X(s) (1 << (s))
There's tons of FSMs where X() is used.
keeping X()
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662/comment/cd103293_e7785c1d PS4, 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
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662/comment/3a713cdc_f3708e26 PS4, 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
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662/comment/83077a4d_41fe4fd7 PS4, 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:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/26662/comment/7cbf2bbd_1130f83e PS4, 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