laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/26823 )
Change subject: Initial IuUP support using proper FSMs ......................................................................
Patch Set 2:
(4 comments)
https://gerrit.osmocom.org/c/osmo-mgw/+/26823/2/include/osmocom/mgcp/mgcp_iu... File include/osmocom/mgcp/mgcp_iuup.h:
https://gerrit.osmocom.org/c/osmo-mgw/+/26823/2/include/osmocom/mgcp/mgcp_iu... PS2, Line 1: / comment wrong?
https://gerrit.osmocom.org/c/osmo-mgw/+/26823/2/src/libosmo-mgcp/mgcp_iuup.c File src/libosmo-mgcp/mgcp_iuup.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/26823/2/src/libosmo-mgcp/mgcp_iuup.c... PS2, Line 28: static struct osmo_iuup_rnl_config def_configure_req = { shouldn't the default be 'const' to avoid it accidentailly being modified/written to?
https://gerrit.osmocom.org/c/osmo-mgw/+/26823/2/src/libosmo-mgcp/mgcp_iuup.c... PS2, Line 229: / should this code be present at all if we're not yet doing the right thing? Or #if0 it ?
https://gerrit.osmocom.org/c/osmo-mgw/+/26823/2/src/libosmo-mgcp/mgcp_iuup.c... PS2, Line 529: / same as the comment above. Do we really want to introduce knonw-broken/insufficient code for the AMR<->IuUP case? Why not introduce just the IuUP<->IuUP case in one patch and then merge the AMR<->IuUP only once it's ready?