dtmf: Make the API more robust and avoid inconsistencies

Because of the issue parsing the MGCP request a '\0' was added to
the queue. This lead to the dtmf scheduler believing that a play
was in progress while the playing code didn't play anything. This
lead to the queue to be stuck and nothing being played at all.

Return the number of tones that should be played and stop using
strlen on the tones.

4 files changed, 19 insertions, 11 deletions

diff --git a/include/dtmf_scheduler.h b/include/dtmf_scheduler.h
index a902466..b1a8b37 100644
--- a/include/dtmf_scheduler.h
+++ b/include/dtmf_scheduler.h
@@ -17,7 +17,7 @@ void dtmf_state_init(struct dtmf_state *state);
 int dtmf_state_add(struct dtmf_state *state, char tone);
 
 /* tones that should be played, playing will be set to 1 */
-void dtmf_state_get_pending(struct dtmf_state *state, char *tones);
+unsigned int dtmf_state_get_pending(struct dtmf_state *state, char *tones);
 
 /* call when the playout is done */
 void dtmf_state_played(struct dtmf_state *state);
diff --git a/src/dtmf_scheduler.c b/src/dtmf_scheduler.c
index a344e76..4f64eb0 100644
--- a/src/dtmf_scheduler.c
+++ b/src/dtmf_scheduler.c
@@ -1,6 +1,6 @@
 /*
- * (C) 2012 by Holger Hans Peter Freyther
- * (C) 2012 by On-Waves
+ * (C) 2012-2013 by Holger Hans Peter Freyther
+ * (C) 2012-2013 by On-Waves
  * All Rights Reserved
  *
  * This program is free software: you can redistribute it and/or modify
@@ -37,7 +37,7 @@ int dtmf_state_add(struct dtmf_state *state, char tone)
 	return 0;
 }
 
-void dtmf_state_get_pending(struct dtmf_state *state, char *tones)
+unsigned int dtmf_state_get_pending(struct dtmf_state *state, char *tones)
 {
 	int pos;
 
@@ -51,6 +51,7 @@ void dtmf_state_get_pending(struct dtmf_state *state, char *tones)
 	if (pos > 0)
 		state->playing = 1;
 	tones[pos] = '\0';
+	return pos;
 }
 
 void dtmf_state_played(struct dtmf_state *state)
diff --git a/src/mgcp_ss7.c b/src/mgcp_ss7.c
index e4d8799..3490a9a 100644
--- a/src/mgcp_ss7.c
+++ b/src/mgcp_ss7.c
@@ -74,18 +74,19 @@ static void play_pending_tones(struct mgcp_endpoint *endp)
 {
 	ToneGenerationT toneGeneration;
 	char tones[25];
+	size_t len;
 
 	/* Check if we need to play anything? */
-	dtmf_state_get_pending(&endp->dtmf_state, tones);
+	len = dtmf_state_get_pending(&endp->dtmf_state, tones);
 
 	/* nothing to play? */
-	if (strlen(tones) == 0)
+	if (len == 0)
 		return;
 
 	/* fill out the data now */
 	osmo_static_assert(sizeof(tones) <= sizeof(toneGeneration.list), Enough_space_for_tones);
 	memset(&toneGeneration, 0, sizeof(toneGeneration));
-	toneGeneration.count = strlen(tones);
+	toneGeneration.count = len;
 	strcpy(toneGeneration.list, tones);
 	MtnSaSetMOB(endp->audio_port, ChannelType_PORT,
 		PredefMob_C_TONE_GENERATION, (char *) &toneGeneration,
diff --git a/tests/dtmf/dtmf_test.c b/tests/dtmf/dtmf_test.c
index 5ffb121..6fab052 100644
--- a/tests/dtmf/dtmf_test.c
+++ b/tests/dtmf/dtmf_test.c
@@ -35,6 +35,7 @@ static void test_queue_while_play(void)
 {
 	struct dtmf_state state;
 	char tone[sizeof(state.tones) + 1];
+	unsigned int len = 0;
 
 	dtmf_state_init(&state);
 
@@ -42,7 +43,8 @@ static void test_queue_while_play(void)
 	ASSERT(dtmf_state_add(&state, 'b'), 0);
 	ASSERT(dtmf_state_add(&state, 'c'), 0);
 
-	dtmf_state_get_pending(&state, tone);
+	len = dtmf_state_get_pending(&state, tone);
+	ASSERT(len, 3);
 	ASSERT(strlen(tone), 3);
 	ASSERT(state.playing, 1);
 	ASSERT(strcmp(tone, "abc"), 0);
@@ -51,7 +53,8 @@ static void test_queue_while_play(void)
 	dtmf_state_played(&state);
 	ASSERT(state.playing, 0);
 
-	dtmf_state_get_pending(&state, tone);
+	len = dtmf_state_get_pending(&state, tone);
+	ASSERT(len, 1);
 	ASSERT(strlen(tone), 1);
 	ASSERT(state.playing, 1);
 	ASSERT(strcmp(tone, "d"), 0);
@@ -61,7 +64,8 @@ static void test_queue_while_play(void)
 	ASSERT(state.playing, 0);
 
 	/* and check that nothing is played */
-	dtmf_state_get_pending(&state, tone);
+	len = dtmf_state_get_pending(&state, tone);
+	ASSERT(len, 0);
 	ASSERT(strlen(tone), 0);
 	ASSERT(state.playing, 0);
 }
@@ -72,6 +76,7 @@ static void test_queue_over_flow(void)
 	const size_t max_items = sizeof(state.tones);
 	char tone[sizeof(state.tones) + 1];
 	int i;
+	unsigned int len;
 
 	dtmf_state_init(&state);
 
@@ -84,7 +89,8 @@ static void test_queue_over_flow(void)
 	ASSERT(dtmf_state_add(&state, 'Z'), -1);
 
 	/* read all of it */
-	dtmf_state_get_pending(&state, tone);
+	len = dtmf_state_get_pending(&state, tone);
+	ASSERT(len, max_items);
 	ASSERT(strlen(tone), max_items);
 	for (i = 0; i < strlen(tone); ++i)
 		ASSERT(tone[i], 'a' + i);