From cb6ad70994dc7686f1c89d803fcca1081b5ef32b Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Tue, 22 Jul 2014 15:00:52 +0200 Subject: [PATCH] mgcp: Change API to remove memory management from the name Jacob pointed out that "free_endp" refers to the memory of the endpoint being freed. What we want is actually a way to release an endpoint (and the resource it allocated) or in the case of the testcase/testapp initialize the data structure correctly. Introduce two names for that. --- openbsc/contrib/testconv/testconv_main.c | 2 +- openbsc/include/openbsc/mgcp.h | 3 ++- openbsc/src/libmgcp/mgcp_protocol.c | 17 +++++++++++------ openbsc/src/libmgcp/mgcp_vty.c | 2 +- openbsc/src/osmo-bsc_mgcp/mgcp_main.c | 2 +- openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c | 6 +++--- openbsc/tests/mgcp/mgcp_test.c | 7 +------ openbsc/tests/mgcp/mgcp_transcoding_test.c | 2 +- 8 files changed, 21 insertions(+), 20 deletions(-) diff --git a/openbsc/contrib/testconv/testconv_main.c b/openbsc/contrib/testconv/testconv_main.c index 89dce1ac2..773be26a3 100644 --- a/openbsc/contrib/testconv/testconv_main.c +++ b/openbsc/contrib/testconv/testconv_main.c @@ -56,7 +56,7 @@ int main(int argc, char **argv) tcfg.endpoints = &endp; tcfg.number_endpoints = 1; endp.tcfg = &tcfg; - mgcp_free_endp(&endp); + mgcp_initialize_endp(&endp); dst_end = &endp.bts_end; src_end = &endp.net_end; diff --git a/openbsc/include/openbsc/mgcp.h b/openbsc/include/openbsc/mgcp.h index 1790f8431..c75637fa7 100644 --- a/openbsc/include/openbsc/mgcp.h +++ b/openbsc/include/openbsc/mgcp.h @@ -225,7 +225,8 @@ int mgcp_parse_config(const char *config_file, struct mgcp_config *cfg, enum mgcp_role role); int mgcp_vty_init(void); int mgcp_endpoints_allocate(struct mgcp_trunk_config *cfg); -void mgcp_free_endp(struct mgcp_endpoint *endp); +void mgcp_release_endp(struct mgcp_endpoint *endp); +void mgcp_initialize_endp(struct mgcp_endpoint *endp); int mgcp_reset_transcoder(struct mgcp_config *cfg); void mgcp_format_stats(struct mgcp_endpoint *endp, char *stats, size_t size); int mgcp_parse_stats(struct msgb *msg, uint32_t *ps, uint32_t *os, uint32_t *pr, uint32_t *_or, int *loss, uint32_t *jitter); diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c index f25c1faa9..1198c24a6 100644 --- a/openbsc/src/libmgcp/mgcp_protocol.c +++ b/openbsc/src/libmgcp/mgcp_protocol.c @@ -940,7 +940,7 @@ mgcp_header_done: if (tcfg->force_realloc) { LOGP(DMGCP, LOGL_NOTICE, "Endpoint 0x%x already allocated. Forcing realloc.\n", ENDPOINT_NUMBER(endp)); - mgcp_free_endp(endp); + mgcp_release_endp(endp); if (p->cfg->realloc_cb) p->cfg->realloc_cb(tcfg, ENDPOINT_NUMBER(endp)); } else { @@ -1006,7 +1006,7 @@ mgcp_header_done: case MGCP_POLICY_REJECT: LOGP(DMGCP, LOGL_NOTICE, "CRCX rejected by policy on 0x%x\n", ENDPOINT_NUMBER(endp)); - mgcp_free_endp(endp); + mgcp_release_endp(endp); return create_err_response(endp, 400, "CRCX", p->trans); break; case MGCP_POLICY_DEFER: @@ -1033,7 +1033,7 @@ mgcp_header_done: create_transcoder(endp); return create_response_with_sdp(endp, "CRCX", p->trans); error2: - mgcp_free_endp(endp); + mgcp_release_endp(endp); LOGP(DMGCP, LOGL_NOTICE, "Resource error on 0x%x\n", ENDPOINT_NUMBER(endp)); return create_err_response(endp, error_code, "CRCX", p->trans); } @@ -1248,7 +1248,7 @@ static struct msgb *handle_delete_con(struct mgcp_parse_data *p) mgcp_format_stats(endp, stats, sizeof(stats)); delete_transcoder(endp); - mgcp_free_endp(endp); + mgcp_release_endp(endp); if (p->cfg->change_cb) p->cfg->change_cb(endp->tcfg, ENDPOINT_NUMBER(endp), MGCP_ENDP_DLCX); @@ -1488,9 +1488,9 @@ int mgcp_endpoints_allocate(struct mgcp_trunk_config *tcfg) return 0; } -void mgcp_free_endp(struct mgcp_endpoint *endp) +void mgcp_release_endp(struct mgcp_endpoint *endp) { - LOGP(DMGCP, LOGL_DEBUG, "Deleting endpoint on: 0x%x\n", ENDPOINT_NUMBER(endp)); + LOGP(DMGCP, LOGL_DEBUG, "Releasing endpoint on: 0x%x\n", ENDPOINT_NUMBER(endp)); endp->ci = CI_UNUSED; endp->allocated = 0; @@ -1516,6 +1516,11 @@ void mgcp_free_endp(struct mgcp_endpoint *endp) memset(&endp->taps, 0, sizeof(endp->taps)); } +void mgcp_initialize_endp(struct mgcp_endpoint *endp) +{ + return mgcp_release_endp(endp); +} + static int send_trans(struct mgcp_config *cfg, const char *buf, int len) { struct sockaddr_in addr; diff --git a/openbsc/src/libmgcp/mgcp_vty.c b/openbsc/src/libmgcp/mgcp_vty.c index 42d99e784..86336680c 100644 --- a/openbsc/src/libmgcp/mgcp_vty.c +++ b/openbsc/src/libmgcp/mgcp_vty.c @@ -1047,7 +1047,7 @@ DEFUN(free_endp, free_endp_cmd, } endp = &trunk->endpoints[endp_no]; - mgcp_free_endp(endp); + mgcp_release_endp(endp); return CMD_SUCCESS; } diff --git a/openbsc/src/osmo-bsc_mgcp/mgcp_main.c b/openbsc/src/osmo-bsc_mgcp/mgcp_main.c index 8c3808a28..16fb72231 100644 --- a/openbsc/src/osmo-bsc_mgcp/mgcp_main.c +++ b/openbsc/src/osmo-bsc_mgcp/mgcp_main.c @@ -181,7 +181,7 @@ static int read_call_agent(struct osmo_fd *fd, unsigned int what) /* is checking in_addr.s_addr == INADDR_LOOPBACK making it more secure? */ for (i = 1; i < reset_trunk->number_endpoints; ++i) - mgcp_free_endp(&reset_trunk->endpoints[i]); + mgcp_release_endp(&reset_trunk->endpoints[i]); } return 0; diff --git a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c index e13827b0b..4a0aaffd7 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c +++ b/openbsc/src/osmo-bsc_nat/bsc_mgcp_utils.c @@ -251,7 +251,7 @@ void bsc_mgcp_free_endpoints(struct bsc_nat *nat) for (i = 1; i < nat->mgcp_cfg->trunk.number_endpoints; ++i){ bsc_mgcp_free_endpoint(nat, i); - mgcp_free_endp(&nat->mgcp_cfg->trunk.endpoints[i]); + mgcp_release_endp(&nat->mgcp_cfg->trunk.endpoints[i]); } } @@ -621,7 +621,7 @@ static void free_chan_downstream(struct mgcp_endpoint *endp, struct bsc_endpoint } bsc_mgcp_free_endpoint(bsc->nat, ENDPOINT_NUMBER(endp)); - mgcp_free_endp(endp); + mgcp_release_endp(endp); } /* @@ -1065,6 +1065,6 @@ void bsc_mgcp_clear_endpoints_for(struct bsc_connection *bsc) rate_ctr_inc(ctr); bsc_mgcp_free_endpoint(bsc->nat, i); - mgcp_free_endp(&bsc->nat->mgcp_cfg->trunk.endpoints[i]); + mgcp_release_endp(&bsc->nat->mgcp_cfg->trunk.endpoints[i]); } } diff --git a/openbsc/tests/mgcp/mgcp_test.c b/openbsc/tests/mgcp/mgcp_test.c index 5f3526650..50c8b2d0a 100644 --- a/openbsc/tests/mgcp/mgcp_test.c +++ b/openbsc/tests/mgcp/mgcp_test.c @@ -828,12 +828,7 @@ static void test_packet_error_detection(int patch_ssrc, int patch_ts) endp.tcfg = &trunk; - /* This doesn't free endp but resets/frees all fields of the structure - * and invokes mgcp_rtp_end_reset() for each mgcp_rtp_end. OTOH, it - * expects valid pointer fields (either NULL or talloc'ed), so the - * memset is still needed. It also requires that endp.tcfg and - * trunk.endpoints are set up properly. */ - mgcp_free_endp(&endp); + mgcp_initialize_endp(&endp); rtp->payload_type = 98; diff --git a/openbsc/tests/mgcp/mgcp_transcoding_test.c b/openbsc/tests/mgcp/mgcp_transcoding_test.c index 2e857a13f..44f307251 100644 --- a/openbsc/tests/mgcp/mgcp_transcoding_test.c +++ b/openbsc/tests/mgcp/mgcp_transcoding_test.c @@ -180,7 +180,7 @@ static int given_configured_endpoint(int in_samples, int out_samples, tcfg->cfg = cfg; endp->tcfg = tcfg; endp->cfg = cfg; - mgcp_free_endp(endp); + mgcp_initialize_endp(endp); dst_end = &endp->bts_end; dst_end->payload_type = audio_name_to_type(dstfmt);