pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/18372 )
Change subject: osmo-mgw: refactor endpoint and trunk handling ...................................................................... Patch Set 3: Code-Review-1 (7 comments) https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3//COMMIT_MSG@30 PS3, Line 30: - get rid of deprecated trunk parameters (leftorvers from the leftovers https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/src/libosmo-mgcp/mgcp_codec.c File src/libosmo-mgcp/mgcp_codec.c: https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/src/libosmo-mgcp/mgcp_codec.c@289 PS3, Line 289: /* FIXME: implement meaningful checks to make sure that the given codec AFAIU we are adding a regression here by dropping this code? https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/src/libosmo-mgcp/mgcp_endp.c File src/libosmo-mgcp/mgcp_endp.c: https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/src/libosmo-mgcp/mgcp_endp.c@100 PS3, Line 100: if (strlen(epname) <= prefix_len) So I was asking about strncmp because I don't know if the 2 strings here are null terminated. If both are null terminated, then it's fine calling directly. https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/src/libosmo-mgcp/mgcp_endp.c@109 PS3, Line 109: return epname; trailing whitespace https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/src/libosmo-mgcp/mgcp_endp.c@216 PS3, Line 216: LOGP(DLMGCP, LOGL_ERROR, "missing domain name in endpoint name \"%s\", expecting '%s'\n", SO you first use double quote for epname, but single quote for cfg->domain? https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/src/libosmo-mgcp/mgcp_endp.c@226 PS3, Line 226: LOGP(DLMGCP, LOGL_ERROR, "wrong domain name in endpoint name \"%s\", expecting '%s'\n", Same https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/tests/mgcp/mgcp_test.c File tests/mgcp/mgcp_test.c: https://gerrit.osmocom.org/c/osmo-mgw/+/18372/3/tests/mgcp/mgcp_test.c@1461 PS3, Line 1461: /* Allocate 5@mgw and let osmo-mgw pick a codec from the list */ I'm still wondering why are you changing the behavior of the test. Can you describe? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/18372 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ice8aaf03faa2fd99074f8665eea3a696d30c5eb3 Gerrit-Change-Number: 18372 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pma...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-Comment-Date: Thu, 28 May 2020 09:25:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
