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? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/26823 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6694a21480b25ab8f35d375295be6601ce38e31d Gerrit-Change-Number: 26823 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier@sysmocom.de> Gerrit-CC: laforge <laforge@osmocom.org> Gerrit-Comment-Date: Mon, 10 Jan 2022 14:06:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment