[PATCH 1/7] unit/test-hfp: Add ability to pass custom result handler to test

[PATCH 1/7] unit/test-hfp: Add ability to pass custom result handler to test

[Szymon Janc]

---
 unit/test-hfp.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/unit/test-hfp.c b/unit/test-hfp.c
index 5218a2b..adc600b 100644
--- a/unit/test-hfp.c
+++ b/unit/test-hfp.c
@@ -51,6 +51,7 @@ struct test_pdu {
 struct test_data {
 	char *test_name;
 	struct test_pdu *pdu_list;
+	hfp_result_func_t result_func;
 };
 
 #define data(args...) ((const unsigned char[]) { args })
@@ -83,7 +84,7 @@ struct test_data {
 		.fragmented = true,				\
 	}
 
-#define define_test(name, function, args...)				\
+#define define_test(name, function, result_function, args...)		\
 	do {								\
 		const struct test_pdu pdus[] = {			\
 			args, { }					\
@@ -91,6 +92,7 @@ struct test_data {
 		static struct test_data data;				\
 		data.test_name = g_strdup(name);			\
 		data.pdu_list = g_malloc(sizeof(pdus));			\
+		data.result_func = result_function;			\
 		memcpy(data.pdu_list, pdus, sizeof(pdus));		\
 		g_test_add_data_func(name, &data, function);		\
 	} while (0)
@@ -252,9 +254,11 @@ static void test_register(gconstpointer data)
 	ret = hfp_gw_set_close_on_unref(context->hfp, true);
 	g_assert(ret);
 
-	ret = hfp_gw_register(context->hfp, prefix_handler, (char *)pdu->data,
-								context, NULL);
-	g_assert(ret);
+	if (context->data->result_func) {
+		ret = hfp_gw_register(context->hfp, context->data->result_func,
+					(char *)pdu->data, context, NULL);
+		g_assert(ret);
+	}
 
 	pdu = &context->data->pdu_list[context->pdu_offset++];
 
@@ -302,42 +306,42 @@ int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
 
-	define_test("/hfp/test_init", test_init, data_end());
-	define_test("/hfp/test_cmd_handler_1", test_command_handler,
+	define_test("/hfp/test_init", test_init, NULL, data_end());
+	define_test("/hfp/test_cmd_handler_1", test_command_handler, NULL,
 			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '\r'),
 			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F'),
 			data_end());
-	define_test("/hfp/test_cmd_handler_2", test_command_handler,
+	define_test("/hfp/test_cmd_handler_2", test_command_handler, NULL,
 			raw_pdu('A', 'T', 'D', '1', '2', '3', '4', '\r'),
 			raw_pdu('A', 'T', 'D', '1', '2', '3', '4'),
 			data_end());
-	define_test("/hfp/test_register_1", test_register,
+	define_test("/hfp/test_register_1", test_register, prefix_handler,
 			raw_pdu('+', 'B', 'R', 'S', 'F', '\0'),
 			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '\r'),
 			type_pdu(HFP_GW_CMD_TYPE_COMMAND, 0),
 			data_end());
-	define_test("/hfp/test_register_2", test_register,
+	define_test("/hfp/test_register_2", test_register, prefix_handler,
 			raw_pdu('+', 'B', 'R', 'S', 'F', '\0'),
 			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '=', '\r'),
 			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
 			data_end());
-	define_test("/hfp/test_register_3", test_register,
+	define_test("/hfp/test_register_3", test_register, prefix_handler,
 			raw_pdu('+', 'B', 'R', 'S', 'F', '\0'),
 			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '?', '\r'),
 			type_pdu(HFP_GW_CMD_TYPE_READ, 0),
 			data_end());
-	define_test("/hfp/test_register_4", test_register,
+	define_test("/hfp/test_register_4", test_register, prefix_handler,
 			raw_pdu('+', 'B', 'R', 'S', 'F', '\0'),
 			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '=', '?',
 									'\r'),
 			type_pdu(HFP_GW_CMD_TYPE_TEST, 0),
 			data_end());
-	define_test("/hfp/test_register_5", test_register,
+	define_test("/hfp/test_register_5", test_register, prefix_handler,
 			raw_pdu('D', '\0'),
 			raw_pdu('A', 'T', 'D', '1', '2', '3', '4', '5', '\r'),
 			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
 			data_end());
-	define_test("/hfp/test_fragmented_1", test_fragmented,
+	define_test("/hfp/test_fragmented_1", test_fragmented, NULL,
 			frg_pdu('A'), frg_pdu('T'), frg_pdu('+'), frg_pdu('B'),
 			frg_pdu('R'), frg_pdu('S'), frg_pdu('F'), frg_pdu('\r'),
 			data_end());
-- 
1.9.0

[PATCH 2/7] unit/test-hfp: Add initial tests for getting unquoted string

[Szymon Janc]

---
 unit/test-hfp.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/unit/test-hfp.c b/unit/test-hfp.c
index adc600b..7837fc3 100644
--- a/unit/test-hfp.c
+++ b/unit/test-hfp.c
@@ -302,6 +302,53 @@ static void test_fragmented(gconstpointer data)
 	execute_context(context);
 }
 
+static void check_ustring_1(struct hfp_gw_result *result,
+				enum hfp_gw_cmd_type type, void *user_data)
+{
+	struct context *context = user_data;
+	const struct test_pdu *pdu;
+	unsigned int i = 3, j = 0;
+	char str[10];
+
+	pdu = &context->data->pdu_list[context->pdu_offset++];
+
+	g_assert(type == pdu->type);
+
+	g_assert(hfp_gw_result_get_unquoted_string(result, str, sizeof(str)));
+
+	while (context->data->pdu_list[1].data[i] != '\r') {
+		g_assert(j < sizeof(str));
+		g_assert(str[j] == context->data->pdu_list[1].data[i]);
+
+		i++;
+		j++;
+	}
+
+	g_assert(str[j] == '\0');
+
+	hfp_gw_send_result(context->hfp, HFP_RESULT_ERROR);
+}
+
+static void check_ustring_2(struct hfp_gw_result *result,
+				enum hfp_gw_cmd_type type, void *user_data)
+{
+	struct context *context = user_data;
+	const struct test_pdu *pdu;
+	char str[10];
+
+	memset(str, 'X', sizeof(str));
+
+	pdu = &context->data->pdu_list[context->pdu_offset++];
+
+	g_assert(type == pdu->type);
+
+	g_assert(!hfp_gw_result_get_unquoted_string(result, str, 3));
+
+	g_assert(str[3] == 'X');
+
+	hfp_gw_send_result(context->hfp, HFP_RESULT_ERROR);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -345,6 +392,16 @@ int main(int argc, char *argv[])
 			frg_pdu('A'), frg_pdu('T'), frg_pdu('+'), frg_pdu('B'),
 			frg_pdu('R'), frg_pdu('S'), frg_pdu('F'), frg_pdu('\r'),
 			data_end());
+	define_test("/hfp/test_ustring_1", test_register, check_ustring_1,
+			raw_pdu('D', '\0'),
+			raw_pdu('A', 'T', 'D', '0', '1', '2', '3', '\r'),
+			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
+			data_end());
+	define_test("/hfp/test_ustring_2", test_register, check_ustring_2,
+			raw_pdu('D', '\0'),
+			raw_pdu('A', 'T', 'D', '0', '1', '2', '3', '\r'),
+			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
+			data_end());
 
 	return g_test_run();
 }
-- 
1.9.0

[PATCH 3/7] unit/test-hfp: Add initial tests for getting quoted string

[Szymon Janc]

---
 unit/test-hfp.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/unit/test-hfp.c b/unit/test-hfp.c
index 7837fc3..445fcb7 100644
--- a/unit/test-hfp.c
+++ b/unit/test-hfp.c
@@ -349,6 +349,54 @@ static void check_ustring_2(struct hfp_gw_result *result,
 	hfp_gw_send_result(context->hfp, HFP_RESULT_ERROR);
 }
 
+static void check_string_1(struct hfp_gw_result *result,
+				enum hfp_gw_cmd_type type, void *user_data)
+{
+	struct context *context = user_data;
+	const struct test_pdu *pdu;
+	unsigned int i = 4, j = 0;
+	char str[10];
+
+	pdu = &context->data->pdu_list[context->pdu_offset++];
+
+	g_assert(type == pdu->type);
+
+	g_assert(hfp_gw_result_get_string(result, str, sizeof(str)));
+
+	while (context->data->pdu_list[1].data[i] != '\"') {
+		g_assert(j < sizeof(str));
+		g_assert(str[j] == context->data->pdu_list[1].data[i]);
+
+		i++;
+		j++;
+	}
+
+	g_assert(context->data->pdu_list[1].data[i] == '\"');
+	g_assert(str[j] == '\0');
+
+	hfp_gw_send_result(context->hfp, HFP_RESULT_ERROR);
+}
+
+static void check_string_2(struct hfp_gw_result *result,
+				enum hfp_gw_cmd_type type, void *user_data)
+{
+	struct context *context = user_data;
+	const struct test_pdu *pdu;
+	char str[10];
+
+	memset(str, 'X', sizeof(str));
+
+	pdu = &context->data->pdu_list[context->pdu_offset++];
+
+	g_assert(type == pdu->type);
+
+	g_assert(!hfp_gw_result_get_string(result, str, 3));
+
+	g_assert(str[3] == 'X');
+
+	hfp_gw_send_result(context->hfp, HFP_RESULT_ERROR);
+}
+
 int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
@@ -402,6 +450,18 @@ int main(int argc, char *argv[])
 			raw_pdu('A', 'T', 'D', '0', '1', '2', '3', '\r'),
 			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
 			data_end());
+	define_test("/hfp/test_string_1", test_register, check_string_1,
+			raw_pdu('D', '\0'),
+			raw_pdu('A', 'T', 'D', '\"', '0', '1', '2', '3', '\"',
+									'\r'),
+			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
+			data_end());
+	define_test("/hfp/test_string_2", test_register, check_string_2,
+			raw_pdu('D', '\0'),
+			raw_pdu('A', 'T', 'D', '\"', '0', '1', '2', '3', '\"',
+									'\r'),
+			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
+			data_end());
 
 	return g_test_run();
 }
-- 
1.9.0

[PATCH 4/7] shared/hfp: Fix not NULL terminating parsed strings

[Szymon Janc]

If there were not enough space in output buffer
hfp_gw_result_get_string() and hfp_gw_result_get_unquoted_string()
wcould return true, but resulting string would not be NULL terminated.
---
 src/shared/hfp.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 1be53fb..e481360 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -308,13 +308,17 @@ bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
 	result->offset++;
 
 	while (data[result->offset] != '\0' && data[result->offset] != '"') {
-		if (i < len)
-			buf[i++] = data[result->offset];
+		if (i == len)
+			return false;
+
+		buf[i++] = data[result->offset];
 		result->offset++;
 	}
 
-	if (i < len)
-		buf[i++] = '\0';
+	if (i == len)
+		return false;
+
+	buf[i] = '\0';
 
 	if (data[result->offset] == '"')
 		result->offset++;
@@ -342,13 +346,17 @@ bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
 
 	while (data[result->offset] != '\0' && data[result->offset] != ','
 					&& data[result->offset] != ')') {
-		if (i < len)
-			buf[i++] = data[result->offset];
+		if (i == len)
+			return false;
+
+		buf[i++] = data[result->offset];
 		result->offset++;
 	}
 
-	if (i < len)
-		buf[i++] = '\0';
+	if (i == len)
+		return false;
+
+	buf[i] = '\0';
 
 	next_field(result);
 
-- 
1.9.0

[PATCH 5/7] shared/hfp: Use unsigned int for offset

[Szymon Janc]

---
 src/shared/hfp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index e481360..3256931 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -72,7 +72,7 @@ struct cmd_handler {
 
 struct hfp_gw_result {
 	const char *data;
-	int offset;
+	unsigned int offset;
 };
 
 static void destroy_cmd_handler(void *data)
@@ -245,8 +245,8 @@ bool hfp_gw_result_get_number_default(struct hfp_gw_result *result,
 
 bool hfp_gw_result_get_number(struct hfp_gw_result *result, unsigned int *val)
 {
+	unsigned int i;
 	int tmp = 0;
-	int i;
 
 	skip_whitespace(result);
 
-- 
1.9.0

[PATCH 6/7] shared/hfp: Don't update offset if string parsing failed

[Szymon Janc]

---
 src/shared/hfp.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 3256931..36c8c3e 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -299,20 +299,22 @@ bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
 {
 	int i = 0;
 	const char *data = result->data;
+	unsigned int offset;
 
 	skip_whitespace(result);
 
 	if (data[result->offset] != '"')
 		return false;
 
-	result->offset++;
+	offset = result->offset;
+	offset++;
 
-	while (data[result->offset] != '\0' && data[result->offset] != '"') {
+	while (data[offset] != '\0' && data[offset] != '"') {
 		if (i == len)
 			return false;
 
-		buf[i++] = data[result->offset];
-		result->offset++;
+		buf[i++] = data[offset];
+		offset++;
 	}
 
 	if (i == len)
@@ -320,11 +322,13 @@ bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
 
 	buf[i] = '\0';
 
-	if (data[result->offset] == '"')
-		result->offset++;
+	if (data[offset] == '"')
+		offset++;
 	else
 		return false;
 
+	result->offset = offset;
+
 	skip_whitespace(result);
 	next_field(result);
 
@@ -335,6 +339,7 @@ bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
 								uint8_t len)
 {
 	const char *data = result->data;
+	unsigned int offset;
 	int i = 0;
 	char c;
 
@@ -344,13 +349,15 @@ bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
 	if (c == '"' || c == ')' || c == '(')
 		return false;
 
-	while (data[result->offset] != '\0' && data[result->offset] != ','
-					&& data[result->offset] != ')') {
+	offset = result->offset;
+
+	while (data[offset] != '\0' && data[offset] != ',' &&
+							data[offset] != ')') {
 		if (i == len)
 			return false;
 
-		buf[i++] = data[result->offset];
-		result->offset++;
+		buf[i++] = data[offset];
+		offset++;
 	}
 
 	if (i == len)
@@ -358,6 +365,8 @@ bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
 
 	buf[i] = '\0';
 
+	result->offset = offset;
+
 	next_field(result);
 
 	return true;
-- 
1.9.0

[PATCH 7/7] doc: Update test-hfp coverage statistics

[Szymon Janc]

---
 doc/test-coverage.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/test-coverage.txt b/doc/test-coverage.txt
index b00ba1d..f6f2de0 100644
--- a/doc/test-coverage.txt
+++ b/doc/test-coverage.txt
@@ -16,7 +16,7 @@ test-mgmt		   2	Management interface handling
 test-textfile		   4	Old textfile storage format
 test-ringbuf		   3	Ring buffer functionality
 test-queue		   1	Queue handling functionality
-test-hfp		   9	HFP Audio Gateway functionality
+test-hfp		  13	HFP Audio Gateway functionality
 test-avdtp		  60	AVDTP qualification test cases
 test-avctp		   9	AVCTP qualification test cases
 test-avrcp		  37	AVRCP qualification test cases
@@ -27,7 +27,7 @@ test-gobex-apparam	  18	OBEX apparam handling
 test-gobex-transfer	  36	OBEX transfer handling
 test-gdbus-client	  12	D-Bus client handling
 			-----
-			 442
+			 446
 
 
 Automated end-to-end testing
-- 
1.9.0

Re: [PATCH 1/7] unit/test-hfp: Add ability to pass custom result handler to test

[Szymon Janc]

On Tuesday 11 March 2014 00:10:28 Szymon Janc wrote:
> ---
>  unit/test-hfp.c | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/unit/test-hfp.c b/unit/test-hfp.c
> index 5218a2b..adc600b 100644
> --- a/unit/test-hfp.c
> +++ b/unit/test-hfp.c
> @@ -51,6 +51,7 @@ struct test_pdu {
>  struct test_data {
>  	char *test_name;
>  	struct test_pdu *pdu_list;
> +	hfp_result_func_t result_func;
>  };
> 
>  #define data(args...) ((const unsigned char[]) { args })
> @@ -83,7 +84,7 @@ struct test_data {
>  		.fragmented = true,				\
>  	}
> 
> -#define define_test(name, function, args...)				\
> +#define define_test(name, function, result_function, args...)		\
>  	do {								\
>  		const struct test_pdu pdus[] = {			\
>  			args, { }					\
> @@ -91,6 +92,7 @@ struct test_data {
>  		static struct test_data data;				\
>  		data.test_name = g_strdup(name);			\
>  		data.pdu_list = g_malloc(sizeof(pdus));			\
> +		data.result_func = result_function;			\
>  		memcpy(data.pdu_list, pdus, sizeof(pdus));		\
>  		g_test_add_data_func(name, &data, function);		\
>  	} while (0)
> @@ -252,9 +254,11 @@ static void test_register(gconstpointer data)
>  	ret = hfp_gw_set_close_on_unref(context->hfp, true);
>  	g_assert(ret);
> 
> -	ret = hfp_gw_register(context->hfp, prefix_handler, (char *)pdu->data,
> -								context, NULL);
> -	g_assert(ret);
> +	if (context->data->result_func) {
> +		ret = hfp_gw_register(context->hfp, context->data->result_func,
> +					(char *)pdu->data, context, NULL);
> +		g_assert(ret);
> +	}
> 
>  	pdu = &context->data->pdu_list[context->pdu_offset++];
> 
> @@ -302,42 +306,42 @@ int main(int argc, char *argv[])
>  {
>  	g_test_init(&argc, &argv, NULL);
> 
> -	define_test("/hfp/test_init", test_init, data_end());
> -	define_test("/hfp/test_cmd_handler_1", test_command_handler,
> +	define_test("/hfp/test_init", test_init, NULL, data_end());
> +	define_test("/hfp/test_cmd_handler_1", test_command_handler, NULL,
>  			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '\r'),
>  			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F'),
>  			data_end());
> -	define_test("/hfp/test_cmd_handler_2", test_command_handler,
> +	define_test("/hfp/test_cmd_handler_2", test_command_handler, NULL,
>  			raw_pdu('A', 'T', 'D', '1', '2', '3', '4', '\r'),
>  			raw_pdu('A', 'T', 'D', '1', '2', '3', '4'),
>  			data_end());
> -	define_test("/hfp/test_register_1", test_register,
> +	define_test("/hfp/test_register_1", test_register, prefix_handler,
>  			raw_pdu('+', 'B', 'R', 'S', 'F', '\0'),
>  			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '\r'),
>  			type_pdu(HFP_GW_CMD_TYPE_COMMAND, 0),
>  			data_end());
> -	define_test("/hfp/test_register_2", test_register,
> +	define_test("/hfp/test_register_2", test_register, prefix_handler,
>  			raw_pdu('+', 'B', 'R', 'S', 'F', '\0'),
>  			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '=', '\r'),
>  			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
>  			data_end());
> -	define_test("/hfp/test_register_3", test_register,
> +	define_test("/hfp/test_register_3", test_register, prefix_handler,
>  			raw_pdu('+', 'B', 'R', 'S', 'F', '\0'),
>  			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '?', '\r'),
>  			type_pdu(HFP_GW_CMD_TYPE_READ, 0),
>  			data_end());
> -	define_test("/hfp/test_register_4", test_register,
> +	define_test("/hfp/test_register_4", test_register, prefix_handler,
>  			raw_pdu('+', 'B', 'R', 'S', 'F', '\0'),
>  			raw_pdu('A', 'T', '+', 'B', 'R', 'S', 'F', '=', '?',
>  									'\r'),
>  			type_pdu(HFP_GW_CMD_TYPE_TEST, 0),
>  			data_end());
> -	define_test("/hfp/test_register_5", test_register,
> +	define_test("/hfp/test_register_5", test_register, prefix_handler,
>  			raw_pdu('D', '\0'),
>  			raw_pdu('A', 'T', 'D', '1', '2', '3', '4', '5', '\r'),
>  			type_pdu(HFP_GW_CMD_TYPE_SET, 0),
>  			data_end());
> -	define_test("/hfp/test_fragmented_1", test_fragmented,
> +	define_test("/hfp/test_fragmented_1", test_fragmented, NULL,
>  			frg_pdu('A'), frg_pdu('T'), frg_pdu('+'), frg_pdu('B'),
>  			frg_pdu('R'), frg_pdu('S'), frg_pdu('F'), frg_pdu('\r'),
>  			data_end());

This is now upstream.

-- 
Szymon K. Janc
szymon.janc@gmail.com