Stefan Sperling has posted comments on this change. ( https://gerrit.osmocom.org/11519 )
Change subject: add aggregated rtp connection stats to osmo-mgw ...................................................................... Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_protocol.c File src/libosmo-mgcp/mgcp_protocol.c: https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_protocol.c@108 PS1, Line 108: static const struct rate_ctr_desc all_rtp_conn_rate_ctr_desc[] = { > Let's better use a macro instead of manually keeping them in sync. That's > going to break eventually. […] I think moving both of these definitions to a shared header file is the best approach. If we keep these two definitions close to each other, the chances of them getting out of sync are insignificant. See patch set 2. Using a macro seems like overkill to me. Apart from the index values which derive from the enum, all the string fields contain different values. And dynamic allocation of rate counter group definitions is discouraged; the rate counter documentation states that definitions are "usually const". -- To view, visit https://gerrit.osmocom.org/11519 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I80d36181600901ae2e0f321dc02b5d54ddc94139 Gerrit-Change-Number: 11519 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Sperling <ssperl...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Stefan Sperling <ssperl...@sysmocom.de> Gerrit-Comment-Date: Wed, 31 Oct 2018 11:43:51 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
