This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/24941 ) Change subject: mgcp_protocol: refactor MGCP request handling ...................................................................... Patch Set 1: (7 comments) https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_msg.c File src/libosmo-mgcp/mgcp_msg.c: https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_msg.c@186 PS1, Line 186: pdata->trans = "000000"; why doing this? it could be that trans was parsed correctly? or we don't use it anyway? https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_protocol.c File src/libosmo-mgcp/mgcp_protocol.c: https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_protocol.c@372 PS1, Line 372: LOGP(DLMGCP, LOGL_ERROR, ERROR? that's expected depending on message type right? print this conditionally based on req.require_endp? EDIT: I see this is already checked further below, so this shouldn't be here afaiu. https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_protocol.c@375 PS1, Line 375: rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_NO_ENDPOINT)); Same, this is not necessarily a failure right? https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_protocol.c@386 PS1, Line 386: rq.mgcp_cause = rc; rc is always 0 here? https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_protocol.c@392 PS1, Line 392: if (pdata.trans && rq.endp->last_trans && strcmp(rq.endp->last_trans, pdata.trans) == 0) { so transactions without endpoint cannot be repeated? because they are under the "rq.endp" condition only. https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_protocol.c@400 PS1, Line 400: if (strncmp(mgcp_requests[i].name, rq.name, 4) == 0) { what about having "name" field be a char[4+1] and copy the first bytes of msgb in there? Looks more safe since you are also passing the pointer in several places and printing it and may be non-null terminated? https://gerrit.osmocom.org/c/osmo-mgw/+/24941/1/src/libosmo-mgcp/mgcp_protocol.c@413 PS1, Line 413: "%s: executing request handler \"%s\" for endpoint resource \"%s\"\n", rq.name, NOTICE here looks a level too high. INFO? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/24941 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I9f519d8a0ee8a513fa1e74acf3ee7dbc0991cdde Gerrit-Change-Number: 24941 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin at sysmocom.de> Gerrit-Comment-Date: Wed, 14 Jul 2021 16:37:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210714/4328886a/attachment.htm>