Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/8054 )

Change subject: a_reset: cleanup + remove dead code
......................................................................


Patch Set 2:

(4 comments)

https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c
File src/libmsc/a_reset.c:

https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c@34
PS2, Line 34: enum fsm_states {
(name is too general, has always been, but not that important; same for fsm_evt)


https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c@90
PS2, Line 90:                .in_event_mask = (1 << EV_CONN_ACK),
As I asked before: we allow repeated Conn ACKs? Is that a real thing?

And even if yes, AFAICT this state doesn't need an action cb, because the 
osmo_fsm_* code already logs the event, and fsm_conn_cb() does nothing but 
trivial logging.


https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c@91
PS2, Line 91:                .out_state_mask = 0,
('.member = 0' can be omitted, happens implicitly)


https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c@125
PS2, Line 125:         reset_fsm = osmo_fsm_inst_alloc(&fsm, NULL, NULL, 
LOGL_DEBUG, name);
use priv arg of osmo_fsm_inst_alloc() (unless you want to use reset_fsm as 
talloc parent for reset_ctx)



--
To view, visit https://gerrit.osmocom.org/8054
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Gerrit-Change-Number: 8054
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pma...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Comment-Date: Mon, 14 May 2018 17:59:03 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Reply via email to