neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31712 ) Change subject: fix coverity (false) warning in codec-list vty ...................................................................... fix coverity (false) warning in codec-list vty Since there were complaints about this old parsing code during recent code review in Ifdc9e04bf1d623da65bfb8a2fddea765601f6d9b, and now also coverity finds something odd in it, just rewrite this. Related: CID#310912 Change-Id: I96cd5d88ec6808a2915c6bccd0c0140216f328f2 --- M src/osmo-bsc/bsc_vty.c M tests/msc.vty 2 files changed, 41 insertions(+), 29 deletions(-) Approvals: fixeria: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c index e14aecb..467301b 100644 --- a/src/osmo-bsc/bsc_vty.c +++ b/src/osmo-bsc/bsc_vty.c @@ -2712,6 +2712,7 @@ { struct bsc_msc_data *data = bsc_msc_data(vty); struct gsm_audio_support tmp[ARRAY_SIZE(data->audio_support)]; + const char *arg = NULL; int i; /* Nr of arguments must fit in the array */ @@ -2724,30 +2725,23 @@ /* check all given arguments first */ for (i = 0; i < argc; i++) { int j; + int ver; + arg = argv[i]; - /* check for hrX or frX */ - if (strlen(argv[i]) != 3 - || argv[i][1] != 'r' - || (argv[i][0] != 'h' && argv[i][0] != 'f') - || argv[i][2] < '0' - || argv[i][2] > '9') { - vty_out(vty, "Codec name must be hrX or frX. Was '%s'%s", argv[i], VTY_NEWLINE); - return CMD_WARNING; - } - - /* store in tmp[] first, to not overwrite data->audio_support[] in case of error */ - tmp[i].ver = atoi(argv[i] + 2); - if (strncmp("hr", argv[i], 2) == 0) + if (strncmp("hr", arg, 2) == 0) tmp[i].hr = 1; - else if (strncmp("fr", argv[i], 2) == 0) + else if (strncmp("fr", arg, 2) == 0) tmp[i].hr = 0; + else + goto invalid_arg; - /* forbid invalid versions */ - if (tmp[i].ver < 1 || tmp[i].ver > 7 - || (tmp[i].hr && tmp[i].ver == 2)) { - vty_out(vty, "'%s' is not a valid codec version%s", argv[i], VTY_NEWLINE); - return CMD_WARNING; - } + ver = atoi(arg + 2); + if (ver < 1 || ver > 7) + goto invalid_arg; + tmp[i].ver = ver; + + if (tmp[i].hr == 1 && ver == 2) + goto invalid_arg; /* prevent duplicate entries */ for (j = 0; j < i; j++) { @@ -2762,6 +2756,10 @@ data->audio_length = argc; return CMD_SUCCESS; + +invalid_arg: + vty_out(vty, "%s is not a valid codec version%s", osmo_quote_cstr_c(OTC_SELECT, arg, -1), VTY_NEWLINE); + return CMD_WARNING; } #define LEGACY_STR "This command has no effect, it is kept to support legacy config files\n" diff --git a/tests/msc.vty b/tests/msc.vty index d67f2b9..08c7f71 100644 --- a/tests/msc.vty +++ b/tests/msc.vty @@ -33,16 +33,16 @@ ... OsmoBSC(config-msc)# codec-list foo -Codec name must be hrX or frX. Was 'foo' +"foo" is not a valid codec version OsmoBSC(config-msc)# codec-list fr10 -Codec name must be hrX or frX. Was 'fr10' +"fr10" is not a valid codec version OsmoBSC(config-msc)# codec-list hr10 -Codec name must be hrX or frX. Was 'hr10' +"hr10" is not a valid codec version OsmoBSC(config-msc)# codec-list FR1 -Codec name must be hrX or frX. Was 'FR1' +"FR1" is not a valid codec version OsmoBSC(config-msc)# # Ensure the codec-list with wrong args did not change the config OsmoBSC(config-msc)# show running-config @@ -62,7 +62,7 @@ ... OsmoBSC(config-msc)# codec-list fr0 fr1 -'fr0' is not a valid codec version +"fr0" is not a valid codec version OsmoBSC(config-msc)# show running-config ... msc 0 @@ -71,7 +71,7 @@ ... OsmoBSC(config-msc)# codec-list hr0 hr1 -'hr0' is not a valid codec version +"hr0" is not a valid codec version OsmoBSC(config-msc)# show running-config ... msc 0 @@ -80,7 +80,7 @@ ... OsmoBSC(config-msc)# codec-list fr8 fr9 -'fr8' is not a valid codec version +"fr8" is not a valid codec version OsmoBSC(config-msc)# show running-config ... msc 0 @@ -89,7 +89,7 @@ ... OsmoBSC(config-msc)# codec-list hr8 hr9 -'hr8' is not a valid codec version +"hr8" is not a valid codec version OsmoBSC(config-msc)# show running-config ... msc 0 @@ -98,7 +98,7 @@ ... OsmoBSC(config-msc)# codec-list fr2 hr2 -'hr2' is not a valid codec version +"hr2" is not a valid codec version OsmoBSC(config-msc)# show running-config ... msc 0 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31712 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I96cd5d88ec6808a2915c6bccd0c0140216f328f2 Gerrit-Change-Number: 31712 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-MessageType: merged