vty: msc / codec-list: forbid duplicate entries

Change-Id: Ifdc9e04bf1d623da65bfb8a2fddea765601f6d9b

include/osmocom/bsc/gsm_data.h

@@ -984,6 +984,8 @@ struct gsm_audio_support {
                 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);

src/osmo-bsc/bsc_vty.c

@@ -2711,42 +2711,50 @@ DEFUN_USRATTR(cfg_net_msc_codec_list,
 	      " (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 %zu entries.%s",
 			argc, 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"

src/osmo-bsc/gsm_data.c

@@ -1042,3 +1042,18 @@ enum rsl_cmod_spd chan_mode_to_rsl_cmod_spd(enum gsm48_chan_mode chan_mode)
 		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);
+}

tests/msc.vty

@@ -53,13 +53,13 @@ msc 0
 ...
 
 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