Patch Set 3: Code-Review-1 (8 comments)
https://gerrit.osmocom.org/#/c/4906/3/include/osmocom/mgcp/mgcp_common.h File include/osmocom/mgcp/mgcp_common.h: Line 73: #define MGCP_CONN_ID_MAXLEN 32+1 (technically this is the SIZE, and MAXLEN would be 32 ... but whatever. In other places we use e.g. GSM_IMSI_LENGTH which contains the +1) https://gerrit.osmocom.org/#/c/4906/3/include/osmocom/mgcp_client/mgcp_client.h File include/osmocom/mgcp_client/mgcp_client.h: Line 30: int response_code; (wow, I messed up indenting when I merged this... not for this patch) https://gerrit.osmocom.org/#/c/4906/3/src/libosmo-mgcp/mgcp_conn.c File src/libosmo-mgcp/mgcp_conn.c: Line 87: OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_MAXLEN); -1: We don't want to assert on that: send an MGCP package with a long conn id and the program crashes. Line 135: OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_MAXLEN); same Line 157: OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_MAXLEN); same (and so forth) https://gerrit.osmocom.org/#/c/4906/3/src/libosmo-mgcp/mgcp_msg.c File src/libosmo-mgcp/mgcp_msg.c: Line 338: "endpoint:%x invalid ConnectionIdentifier (missing) %s\n", let's log with 0x prefix to clarify that it is hex: "endpoint 0x%x: invalid..." same below Line 339: ENDPOINT_NUMBER(endp), conn_id); conn_id is NULL and you're logging it? Line 347: ENDPOINT_NUMBER(endp), conn_id); conn_id is empty and you're logging it? -- To view, visit https://gerrit.osmocom.org/4906 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0531a1b670d00cec50078423a2868207135b2436 Gerrit-PatchSet: 3 Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Owner: Harald Welte <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: dexter <[email protected]> Gerrit-HasComments: Yes
