neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/31610 )


Change subject: vty: msc / codec-list: forbid duplicate entries
......................................................................

vty: msc / codec-list: forbid duplicate entries

Change-Id: Ifdc9e04bf1d623da65bfb8a2fddea765601f6d9b
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/gsm_data.c
M tests/msc.vty
4 files changed, 57 insertions(+), 23 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/10/31610/1

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 8c4dcce..b2aa590 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -984,6 +984,8 @@
                 ver : 7;
 };

+int gsm_audio_support_cmp(const struct gsm_audio_support *a, const struct 
gsm_audio_support *b);
+
 extern void talloc_ctx_init(void *ctx_root);

 enum gsm_bts_type parse_btstype(const char *arg);
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index bea85bb..fdb61e2 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -2711,42 +2711,50 @@
              " (fr3: AMR-FR, hr3: AMR-HR, fr2: GSM-EFR, fr1: GSM-FR, hr1: 
GSM-HR)\n")
 {
        struct bsc_msc_data *data = bsc_msc_data(vty);
+       struct gsm_audio_support tmp[ARRAY_SIZE(data->audio_support)];
        int i;

-       /* check all given arguments first */
-       for (i = 0; i < argc; 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')
-                       goto error;
-       }
-
+       /* Nr of arguments must fit in the array */
        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;
        }

-       data->audio_length = argc;
+       /* check all given arguments first */
+       for (i = 0; i < argc; i++) {
+               int j;

-       for (i = 0; i < argc; ++i) {
-               data->audio_support[i].ver = atoi(argv[i] + 2);
+               /* 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)
-                       data->audio_support[i].hr = 1;
+                       tmp[i].hr = 1;
                else if (strncmp("fr", argv[i], 2) == 0)
-                       data->audio_support[i].hr = 0;
+                       tmp[i].hr = 0;
+
+               /* prevent duplicate entries */
+               for (j = 0; j < i; j++) {
+                       if (gsm_audio_support_cmp(&tmp[j], &tmp[i]) == 0) {
+                               vty_out(vty, "duplicate entry in 'msc' / 
'codec-support': %s%s", argv[i], VTY_NEWLINE);
+                               return CMD_WARNING;
+                       }
+               }
        }

-       return CMD_SUCCESS;
+       memcpy(data->audio_support, tmp, sizeof(data->audio_support));
+       data->audio_length = argc;

-error:
-       vty_out(vty, "Codec name must be hrX or frX. Was '%s'%s",
-                       argv[i], VTY_NEWLINE);
-       return CMD_WARNING;
+       return CMD_SUCCESS;
 }

 #define LEGACY_STR "This command has no effect, it is kept to support legacy 
config files\n"
diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c
index 1562ea8..a682a32 100644
--- a/src/osmo-bsc/gsm_data.c
+++ b/src/osmo-bsc/gsm_data.c
@@ -1042,3 +1042,18 @@
                return -EINVAL;
        }
 }
+
+int gsm_audio_support_cmp(const struct gsm_audio_support *a, const struct 
gsm_audio_support *b)
+{
+       int rc;
+       if (a == b)
+               return 0;
+       if (!a)
+               return -1;
+       if (!b)
+               return 1;
+       rc = OSMO_CMP(a->hr, b->hr);
+       if (rc)
+               return rc;
+       return OSMO_CMP(a->ver, b->ver);
+}
diff --git a/tests/msc.vty b/tests/msc.vty
index af4c33e..8cbcb83 100644
--- a/tests/msc.vty
+++ b/tests/msc.vty
@@ -53,13 +53,13 @@
 ...

 OsmoBSC(config-msc)# codec-list fr1 fr1
+duplicate entry in 'msc' / 'codec-support': fr1
 OsmoBSC(config-msc)# show running-config
 ...
 msc 0
 ...
- codec-list fr1 fr1
+ codec-list hr1 hr3 fr1 fr2 fr3
 ...
-OsmoBSC(config-msc)# # ERROR: duplicate entries make no sense

 OsmoBSC(config-msc)# codec-list fr0 fr1
 OsmoBSC(config-msc)# show running-config

--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31610
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ifdc9e04bf1d623da65bfb8a2fddea765601f6d9b
Gerrit-Change-Number: 31610
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-MessageType: newchange

Reply via email to