neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31609 ) Change subject: simplify storage of bsc_msc_data->audio_support ...................................................................... simplify storage of bsc_msc_data->audio_support Make it a plain array, no dynamic allocation needed. Change-Id: I625cedc4bb040d649fd6e1794ba468f4c6ad6adc --- M include/osmocom/bsc/bsc_msc_data.h M src/osmo-bsc/bsc_vty.c M src/osmo-bsc/codec_pref.c M src/osmo-bsc/osmo_bsc_msc.c M tests/codec_pref/codec_pref_test.c M tests/msc.vty 6 files changed, 87 insertions(+), 130 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/09/31609/1 diff --git a/include/osmocom/bsc/bsc_msc_data.h b/include/osmocom/bsc/bsc_msc_data.h index 3568716..31e9975 100644 --- a/include/osmocom/bsc/bsc_msc_data.h +++ b/include/osmocom/bsc/bsc_msc_data.h @@ -133,8 +133,13 @@ /* audio codecs */ struct gsm48_multi_rate_conf amr_conf; bool amr_octet_aligned; - struct gsm_audio_support **audio_support; + + /* Practically, there can be at most 5 entries in osmo-bsc: FR1 FR2 FR3 HR1 HR3. Historically, osmo-bsc allowed + * *any* number of entries -- we should not break too strictly on old cfg files. Theoretically, there should be + * room for FR1 to FR7 plus HR1 to HR7, i.e. 14. Let's just make it: */ + struct gsm_audio_support audio_support[16]; int audio_length; + enum bsc_lcls_mode lcls_mode; bool lcls_codec_mismatch_allow; diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c index bba84e2..bea85bb 100644 --- a/src/osmo-bsc/bsc_vty.c +++ b/src/osmo-bsc/bsc_vty.c @@ -2550,10 +2550,10 @@ if (i != 0) vty_out(vty, " "); - if (msc->audio_support[i]->hr) - vty_out(vty, "hr%u", msc->audio_support[i]->ver); + if (msc->audio_support[i].hr) + vty_out(vty, "hr%u", msc->audio_support[i].ver); else - vty_out(vty, "fr%u", msc->audio_support[i]->ver); + vty_out(vty, "fr%u", msc->audio_support[i].ver); } vty_out(vty, "%s", VTY_NEWLINE); @@ -2724,27 +2724,21 @@ goto error; } - /* free the old list... if it exists */ - if (data->audio_support) { - talloc_free(data->audio_support); - data->audio_support = NULL; - data->audio_length = 0; + if (argc > ARRAY_SIZE(data->audio_support)) { + vty_out(vty, "Too many items in 'msc' / 'codec-list': %d. There can be at most %d entries.%s", + argc, (int)ARRAY_SIZE(data->audio_support), VTY_NEWLINE); + return CMD_ERR_EXEED_ARGC_MAX; } - /* create a new array */ - data->audio_support = - talloc_zero_array(bsc_gsmnet, struct gsm_audio_support *, argc); data->audio_length = argc; for (i = 0; i < argc; ++i) { - data->audio_support[i] = talloc_zero(data->audio_support, - struct gsm_audio_support); - data->audio_support[i]->ver = atoi(argv[i] + 2); + data->audio_support[i].ver = atoi(argv[i] + 2); if (strncmp("hr", argv[i], 2) == 0) - data->audio_support[i]->hr = 1; + data->audio_support[i].hr = 1; else if (strncmp("fr", argv[i], 2) == 0) - data->audio_support[i]->hr = 0; + data->audio_support[i].hr = 0; } return CMD_SUCCESS; diff --git a/src/osmo-bsc/codec_pref.c b/src/osmo-bsc/codec_pref.c index a64425a..dc29a5a 100644 --- a/src/osmo-bsc/codec_pref.c +++ b/src/osmo-bsc/codec_pref.c @@ -333,7 +333,7 @@ * indeed available with the current BTS and MSC configuration */ for (i = 0; i < msc->audio_length; i++) { /* Pick a permitted speech value from the global codec configuration list */ - perm_spch = audio_support_to_gsm88(msc->audio_support[i]); + perm_spch = audio_support_to_gsm88(&msc->audio_support[i]); /* Determine if the result is a half or full rate codec */ rc = full_rate_from_perm_spch(&full_rate, perm_spch); @@ -404,7 +404,7 @@ for (i = 0; i < msc->audio_length; i++) { /* Pick a permitted speech value from the global codec configuration list */ - perm_spch = audio_support_to_gsm88(msc->audio_support[i]); + perm_spch = audio_support_to_gsm88(&msc->audio_support[i]); /* Check this permitted speech value against the BTS specific parameters. * if the BTS does not support the codec, try the next one */ @@ -419,8 +419,8 @@ /* AMR (HR/FR version 3) is the only codec that requires a codec * configuration (S0-S15). Determine the current configuration and update * the cfg flag. */ - if (msc->audio_support[i]->ver == 3) - scl->codec[scl->len].cfg = gen_bss_supported_amr_s15_s0(msc, bts, msc->audio_support[i]->hr); + if (msc->audio_support[i].ver == 3) + scl->codec[scl->len].cfg = gen_bss_supported_amr_s15_s0(msc, bts, msc->audio_support[i].hr); scl->len++; } diff --git a/src/osmo-bsc/osmo_bsc_msc.c b/src/osmo-bsc/osmo_bsc_msc.c index 4d5d5b2..08a2466 100644 --- a/src/osmo-bsc/osmo_bsc_msc.c +++ b/src/osmo-bsc/osmo_bsc_msc.c @@ -207,7 +207,6 @@ struct bsc_msc_data *osmo_msc_data_alloc(struct gsm_network *net, int nr) { struct bsc_msc_data *msc_data; - unsigned int i; /* check if there is already one */ msc_data = osmo_msc_data_find(net, nr); @@ -251,24 +250,16 @@ /* Allow the full set of possible codecs by default */ msc_data->audio_length = 5; - msc_data->audio_support = - talloc_zero_array(msc_data, struct gsm_audio_support *, - msc_data->audio_length); - for (i = 0; i < msc_data->audio_length; i++) { - msc_data->audio_support[i] = - talloc_zero(msc_data->audio_support, - struct gsm_audio_support); - } - msc_data->audio_support[0]->ver = 1; - msc_data->audio_support[0]->hr = 0; - msc_data->audio_support[1]->ver = 1; - msc_data->audio_support[1]->hr = 1; - msc_data->audio_support[2]->ver = 2; - msc_data->audio_support[2]->hr = 0; - msc_data->audio_support[3]->ver = 3; - msc_data->audio_support[3]->hr = 0; - msc_data->audio_support[4]->ver = 3; - msc_data->audio_support[4]->hr = 1; + msc_data->audio_support[0].ver = 1; + msc_data->audio_support[0].hr = 0; + msc_data->audio_support[1].ver = 1; + msc_data->audio_support[1].hr = 1; + msc_data->audio_support[2].ver = 2; + msc_data->audio_support[2].hr = 0; + msc_data->audio_support[3].ver = 3; + msc_data->audio_support[3].hr = 0; + msc_data->audio_support[4].ver = 3; + msc_data->audio_support[4].hr = 1; osmo_fd_setup(&msc_data->mgcp_ipa.ofd, -1, OSMO_FD_READ, &bsc_sccplite_mgcp_proxy_cb, msc_data, 0); msc_data->mgcp_ipa.local_addr = NULL; /* = INADDR(6)_ANY */ diff --git a/tests/codec_pref/codec_pref_test.c b/tests/codec_pref/codec_pref_test.c index 5f4c831..920352c 100644 --- a/tests/codec_pref/codec_pref_test.c +++ b/tests/codec_pref/codec_pref_test.c @@ -30,27 +30,8 @@ void *ctx = NULL; -#define MSC_AUDIO_SUPPORT_MAX 5 #define N_CONFIG_VARIANTS 9 -/* Make sure that there is some memory to put our test configuration. */ -static void init_msc_config(struct bsc_msc_data *msc) -{ - unsigned int i; - - msc->audio_support = talloc_zero_array(ctx, struct gsm_audio_support *, MSC_AUDIO_SUPPORT_MAX); - msc->audio_length = MSC_AUDIO_SUPPORT_MAX; - for (i = 0; i < MSC_AUDIO_SUPPORT_MAX; i++) { - msc->audio_support[i] = talloc_zero(msc->audio_support, struct gsm_audio_support); - } -} - -/* Free memory that we have used for the test configuration. */ -static void free_msc_config(struct bsc_msc_data *msc) -{ - talloc_free(msc->audio_support); -} - /* The speech codec list is sent by the MS and lists the voice codec settings * that the MS is able to support. The BSC must select one of this codecs * depending on what the MSC is able to support. The following function @@ -215,74 +196,74 @@ switch (config_no) { case 0: /* FR1 only */ - msc->audio_support[0]->ver = 1; - msc->audio_support[0]->hr = 0; + msc->audio_support[0].ver = 1; + msc->audio_support[0].hr = 0; msc->audio_length = 1; break; case 1: /* HR1 only */ - msc->audio_support[0]->ver = 1; - msc->audio_support[0]->hr = 1; + msc->audio_support[0].ver = 1; + msc->audio_support[0].hr = 1; msc->audio_length = 1; break; case 2: /* FR2 only */ - msc->audio_support[0]->ver = 2; - msc->audio_support[0]->hr = 0; + msc->audio_support[0].ver = 2; + msc->audio_support[0].hr = 0; msc->audio_length = 1; break; case 3: /* FR3 only */ - msc->audio_support[0]->ver = 3; - msc->audio_support[0]->hr = 0; + msc->audio_support[0].ver = 3; + msc->audio_support[0].hr = 0; msc->audio_length = 1; break; case 4: /* HR3 only */ - msc->audio_support[0]->ver = 3; - msc->audio_support[0]->hr = 1; + msc->audio_support[0].ver = 3; + msc->audio_support[0].hr = 1; msc->audio_length = 1; break; case 5: /* FR1 and HR1 */ - msc->audio_support[0]->ver = 1; - msc->audio_support[0]->hr = 0; - msc->audio_support[1]->ver = 1; - msc->audio_support[1]->hr = 1; + msc->audio_support[0].ver = 1; + msc->audio_support[0].hr = 0; + msc->audio_support[1].ver = 1; + msc->audio_support[1].hr = 1; msc->audio_length = 2; break; case 6: /* FR1, FR2 and HR1 */ - msc->audio_support[0]->ver = 1; - msc->audio_support[0]->hr = 0; - msc->audio_support[1]->ver = 2; - msc->audio_support[1]->hr = 0; - msc->audio_support[2]->ver = 1; - msc->audio_support[2]->hr = 1; + msc->audio_support[0].ver = 1; + msc->audio_support[0].hr = 0; + msc->audio_support[1].ver = 2; + msc->audio_support[1].hr = 0; + msc->audio_support[2].ver = 1; + msc->audio_support[2].hr = 1; msc->audio_length = 3; break; case 7: /* FR1, FR3 and HR3 */ - msc->audio_support[0]->ver = 1; - msc->audio_support[0]->hr = 0; - msc->audio_support[1]->ver = 3; - msc->audio_support[1]->hr = 0; - msc->audio_support[2]->ver = 3; - msc->audio_support[2]->hr = 1; + msc->audio_support[0].ver = 1; + msc->audio_support[0].hr = 0; + msc->audio_support[1].ver = 3; + msc->audio_support[1].hr = 0; + msc->audio_support[2].ver = 3; + msc->audio_support[2].hr = 1; msc->audio_length = 3; break; case 8: /* FR1, FR2, FR3, HR1 and HR3 */ - msc->audio_support[0]->ver = 1; - msc->audio_support[0]->hr = 0; - msc->audio_support[1]->ver = 2; - msc->audio_support[1]->hr = 0; - msc->audio_support[2]->ver = 3; - msc->audio_support[2]->hr = 0; - msc->audio_support[3]->ver = 1; - msc->audio_support[3]->hr = 1; - msc->audio_support[4]->ver = 3; - msc->audio_support[4]->hr = 1; + msc->audio_support[0].ver = 1; + msc->audio_support[0].hr = 0; + msc->audio_support[1].ver = 2; + msc->audio_support[1].hr = 0; + msc->audio_support[2].ver = 3; + msc->audio_support[2].hr = 0; + msc->audio_support[3].ver = 1; + msc->audio_support[3].hr = 1; + msc->audio_support[4].ver = 3; + msc->audio_support[4].hr = 1; msc->audio_length = 5; break; } @@ -394,10 +375,10 @@ printf(" * BSS: audio support settings (%u items):\n", msc->audio_length); for (i = 0; i < msc->audio_length; i++) - if (msc->audio_support[i]->hr) - printf(" audio_support[%u]=HR%u\n", i, msc->audio_support[i]->ver); + if (msc->audio_support[i].hr) + printf(" audio_support[%u]=HR%u\n", i, msc->audio_support[i].ver); else - printf(" audio_support[%u]=FR%u\n", i, msc->audio_support[i]->ver); + printf(" audio_support[%u]=FR%u\n", i, msc->audio_support[i].ver); printf(" * BTS: audio support settings:\n"); printf(" (GSM-FR implicitly supported)\n"); @@ -426,8 +407,6 @@ printf("============== test_one_to_one ==============\n\n"); - init_msc_config(&msc_local); - for (i = 0; i < N_CONFIG_VARIANTS; i++) { make_msc_config(&msc_local, i); make_scl_config(&scl_ms, i); @@ -436,8 +415,6 @@ rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local, &bts_local); OSMO_ASSERT(rc == 0); } - - free_msc_config(&msc_local); } /* Network supports all combinations, MS varies */ @@ -452,8 +429,6 @@ printf("============== test_ms ==============\n\n"); - init_msc_config(&msc_local); - make_msc_config(&msc_local, 8); make_ct_config(&ct_msc, 8); make_bts_config(&bts_local, 8); @@ -462,8 +437,6 @@ rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local, &bts_local); OSMO_ASSERT(rc == 0); } - - free_msc_config(&msc_local); } /* BSS and MS support all combinations, MSC varies */ @@ -478,8 +451,6 @@ printf("============== test_ct ==============\n\n"); - init_msc_config(&msc_local); - make_msc_config(&msc_local, 8); make_scl_config(&scl_ms, 8); make_bts_config(&bts_local, 8); @@ -488,8 +459,6 @@ rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local, &bts_local); OSMO_ASSERT(rc == 0); } - - free_msc_config(&msc_local); } /* MSC and MS support all combinations, BSS varies */ @@ -504,8 +473,6 @@ printf("============== test_msc ==============\n\n"); - init_msc_config(&msc_local); - make_ct_config(&ct_msc, 8); make_scl_config(&scl_ms, 8); make_bts_config(&bts_local, 8); @@ -514,8 +481,6 @@ rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local, &bts_local); OSMO_ASSERT(rc == 0); } - - free_msc_config(&msc_local); } /* Some mixed configurations that are supposed to work */ @@ -529,8 +494,6 @@ printf("============== test_selected_working ==============\n\n"); - init_msc_config(&msc_local); - make_scl_config(&scl_ms, 6); make_ct_config(&ct_msc, 5); make_msc_config(&msc_local, 7); @@ -572,8 +535,6 @@ make_bts_config(&bts_local, 1); rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local, &bts_local); OSMO_ASSERT(rc == 0); - - free_msc_config(&msc_local); } /* Some mixed configurations that can not work */ @@ -587,8 +548,6 @@ printf("============== test_selected_non_working ==============\n\n"); - init_msc_config(&msc_local); - make_scl_config(&scl_ms, 1); make_ct_config(&ct_msc, 5); make_msc_config(&msc_local, 7); @@ -630,8 +589,6 @@ make_bts_config(&bts_local, 7); rc = test_match_codec_pref(&ct_msc, &scl_ms, &msc_local, &bts_local); OSMO_ASSERT(rc == -1); - - free_msc_config(&msc_local); } /* Try execute bss_supp_codec_list(), display input and output parameters */ @@ -644,10 +601,10 @@ printf(" * BSS: audio support settings (%u items):\n", msc->audio_length); for (i = 0; i < msc->audio_length; i++) - if (msc->audio_support[i]->hr) - printf(" audio_support[%u]=HR%u\n", i, msc->audio_support[i]->ver); + if (msc->audio_support[i].hr) + printf(" audio_support[%u]=HR%u\n", i, msc->audio_support[i].ver); else - printf(" audio_support[%u]=FR%u\n", i, msc->audio_support[i]->ver); + printf(" audio_support[%u]=FR%u\n", i, msc->audio_support[i].ver); printf(" * BTS: audio support settings:\n"); printf(" (GSM-FR implicitly supported)\n"); @@ -660,7 +617,7 @@ printf(" * result: speech codec list (%u items):\n", scl.len); for (i = 0; i < scl.len; i++) { printf(" codec[%u]->type=%s", i, gsm0808_speech_codec_type_name(scl.codec[i].type)); - if (msc->audio_support[i]->ver == 3) + if (msc->audio_support[i].ver == 3) printf(" S15-S0=%04x", scl.codec[i].cfg); printf("\n"); } @@ -676,8 +633,6 @@ uint8_t k; printf("============== test_gen_bss_supp_codec_list_cfgs ==============\n\n"); - init_msc_config(&msc_local); - for (i = 0; i < N_CONFIG_VARIANTS; i++) { for (k = 0; k < N_CONFIG_VARIANTS; k++) { make_msc_config(&msc_local, i); @@ -686,8 +641,6 @@ test_gen_bss_supported_codec_list(&msc_local, &bts_local); } } - - free_msc_config(&msc_local); } static const struct log_info_cat log_categories[] = { diff --git a/tests/msc.vty b/tests/msc.vty index 56a3fe2..af4c33e 100644 --- a/tests/msc.vty +++ b/tests/msc.vty @@ -114,3 +114,6 @@ codec-list fr1 fr2 fr3 fr4 ... OsmoBSC(config-msc)# # TODO: should fr4 thru fr7 be rejected + +OsmoBSC(config-msc)# codec-list fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 fr1 +Too many items in 'msc' / 'codec-list': 17. There can be at most 16 entries. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31609 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I625cedc4bb040d649fd6e1794ba468f4c6ad6adc Gerrit-Change-Number: 31609 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr@sysmocom.de> Gerrit-MessageType: newchange