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/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Patch Set 3: Code-Review-1
(8 comments)
excellent improvements. I still found two problems, a couple instances where I can accept if you oppose my previous comments but would like a comment on it if so. And few minor cosmetics.
https://gerrit.osmocom.org/#/c/4980/3/src/libmsc/msc_mgcp.c
File src/libmsc/msc_mgcp.c:
Line 65: static const struct value_string msc_mgcp_cause_codes_str[] = {
(prefix 'msc_mgcp_': nice. Why not the common value_string[] suffix '_names'?)
Line 283: "CRCX/CN creating connection for the CN side on MGW endpoint:%0x%x...\n", mgcp_ctx->rtp_endpoint);
-1: oops: %0x%x
Line 389: OSMO_ASSERT(false);
well, or we added a new RAN type and forgot to implement it here. I usually do error log like "Unknown RAN type: %d" and returning an error.
Line 445: conn->rtp.remote_port_cn);
(i.e. you don't want to combine these two logs?)
Line 529: conn->rtp.remote_port_ran);
(i.e. you don't want to combine these two logs?)
Line 803: mgcp_ctx->fsm = osmo_fsm_inst_alloc(&fsm_msc_mgcp, NULL, NULL, LOGL_DEBUG, name);
i.e. you don't want the FSM to be a child of the conn_fsm?
Line 806: LOGPFSML(mgcp_ctx->fsm, LOGL_DEBUG, "MGW handler fsm created\n");
(this is also a duplicate of FSM internal logging, right?)
Line 858: LOGP(DMGCP, LOGL_ERROR, "invalid call state, call completion failed\n");
-1: context
--
To view, visit https://gerrit.osmocom.org/4980
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieea9630358b3963261fa1993cf1f3b563ff23538
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes