[PATCH v2 1/2] stkutil: Complete the TLV parsing/builder to support BIP commands

[PATCH v2 1/2] stkutil: Complete the TLV parsing/builder to support BIP commands

[Philippe Nunes]

[-- Attachment #1: Type: text/plain, Size: 20117 bytes --]

---
 src/stkutil.c |  287 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 src/stkutil.h |  132 ++++++++++++++++++++++-----
 2 files changed, 380 insertions(+), 39 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index c64cb7a..94be192 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -1264,8 +1264,20 @@ static gboolean parse_dataobj_bearer_description(
 
 	data = comprehension_tlv_iter_get_data(iter);
 	bd->type = data[0];
-	bd->len = len - 1;
-	memcpy(bd->pars, data + 1, bd->len);
+
+	/* Parse only the packet data service bearer parameters */
+	if (bd->type != STK_BEARER_TYPE_GPRS_UTRAN)
+		return FALSE;
+
+	if (len < 7)
+		return FALSE;
+
+	bd->gprs.precedence = data[1];
+	bd->gprs.delay = data[2];
+	bd->gprs.reliability = data[3];
+	bd->gprs.peak = data[4];
+	bd->gprs.mean = data[5];
+	bd->gprs.pdp_type = data[6];
 
 	return TRUE;
 }
@@ -1355,8 +1367,16 @@ static gboolean parse_dataobj_other_address(
 		return FALSE;
 
 	data = comprehension_tlv_iter_get_data(iter);
+
+	if (data[0] != STK_ADDRESS_IPV4 && data[0] != STK_ADDRESS_IPV6)
+		return FALSE;
+
 	oa->type = data[0];
-	memcpy(&oa->addr, data + 1, len - 1);
+
+	if (oa->type == STK_ADDRESS_IPV4)
+		memcpy(&oa->addr.ipv4, data + 1, 4);
+	else
+		memcpy(&oa->addr.ipv6, data + 1, 16);
 
 	return TRUE;
 }
@@ -1604,16 +1624,48 @@ static gboolean parse_dataobj_esn(struct comprehension_tlv_iter *iter,
 static gboolean parse_dataobj_network_access_name(
 		struct comprehension_tlv_iter *iter, void *user)
 {
-	struct stk_network_access_name *nan = user;
+	char **apn = user;
 	const unsigned char *data;
 	unsigned int len = comprehension_tlv_iter_get_length(iter);
+	unsigned char label_size;
+	unsigned char offset = 0;
+	char decoded_apn[100];
 
-	if (len == 0)
+	if (len == 0 || len > 100)
 		return FALSE;
 
 	data = comprehension_tlv_iter_get_data(iter);
-	nan->len = len;
-	memcpy(nan->name, data, len);
+	/*
+	 * As specified in TS 23 003 Section 9
+	 * The APN consists of one or more labels. Each label is coded as
+	 * a one octet length field followed by that number of octets coded
+	 * as 8 bit ASCII characters
+	 */
+
+	while (len) {
+		label_size = *data;
+
+		if (label_size > (len-1))
+			return FALSE;
+
+		memcpy(decoded_apn + offset, data + 1, label_size);
+
+		data += label_size + 1;
+		offset += label_size;
+
+		len -= label_size + 1;
+		if (len) {
+			decoded_apn[offset] = '.';
+			offset += 1;
+		}
+	}
+
+	*apn = g_try_malloc(offset + 1);
+	if (*apn == NULL)
+		return FALSE;
+
+	memcpy(*apn, decoded_apn, offset);
+	(*apn)[offset] = '\0';
 
 	return TRUE;
 }
@@ -3274,7 +3326,68 @@ static enum stk_command_parse_result parse_launch_browser(
 				STK_DATA_OBJECT_TYPE_INVALID);
 }
 
-/* TODO: parse_open_channel */
+static void destroy_open_channel(struct stk_command *command)
+{
+	g_free(command->open_channel.alpha_id);
+	g_free(command->open_channel.text_usr);
+	g_free(command->open_channel.text_passwd);
+}
+
+static enum stk_command_parse_result parse_open_channel(
+					struct stk_command *command,
+					struct comprehension_tlv_iter *iter)
+{
+	struct stk_command_open_channel *obj = &command->open_channel;
+	enum stk_command_parse_result status;
+
+	if (command->qualifier >= 0x08)
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_open_channel;
+
+	/*
+	 * parse the Open Channel data objects related to packet data service
+	 * bearer
+	 */
+	status = parse_dataobj(iter,
+				STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+				&obj->alpha_id,
+				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
+				&obj->icon_id,
+				STK_DATA_OBJECT_TYPE_BEARER_DESCRIPTION,
+				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
+				&obj->bearer_desc,
+				STK_DATA_OBJECT_TYPE_BUFFER_SIZE,
+				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
+				&obj->buf_size,
+				STK_DATA_OBJECT_TYPE_NETWORK_ACCESS_NAME, 0,
+				&obj->apn,
+				STK_DATA_OBJECT_TYPE_OTHER_ADDRESS, 0,
+				&obj->local_addr,
+				STK_DATA_OBJECT_TYPE_TEXT, 0,
+				&obj->text_usr,
+				STK_DATA_OBJECT_TYPE_TEXT, 0,
+				&obj->text_passwd,
+				STK_DATA_OBJECT_TYPE_UICC_TE_INTERFACE, 0,
+				&obj->uti,
+				STK_DATA_OBJECT_TYPE_OTHER_ADDRESS, 0,
+				&obj->data_dest_addr,
+				STK_DATA_OBJECT_TYPE_TEXT_ATTRIBUTE, 0,
+				&obj->text_attr,
+				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
+				&obj->frame_id,
+				STK_DATA_OBJECT_TYPE_INVALID);
+
+	CHECK_TEXT_AND_ICON(obj->alpha_id, obj->icon_id.id);
+
+	return status;
+}
 
 static void destroy_close_channel(struct stk_command *command)
 {
@@ -3291,7 +3404,8 @@ static enum stk_command_parse_result parse_close_channel(
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
 		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
+	if ((command->dst < STK_DEVICE_IDENTITY_TYPE_CHANNEL_1) ||
+			(command->dst > STK_DEVICE_IDENTITY_TYPE_CHANNEL_7))
 		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	command->destructor = destroy_close_channel;
@@ -3363,6 +3477,9 @@ static enum stk_command_parse_result parse_send_data(
 	struct stk_command_send_data *obj = &command->send_data;
 	enum stk_command_parse_result status;
 
+	if (command->qualifier > STK_SEND_DATA_IMMEDIATELY)
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
 		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
@@ -3737,6 +3854,8 @@ static enum stk_command_parse_result parse_command_body(
 		return parse_language_notification(command, iter);
 	case STK_COMMAND_TYPE_LAUNCH_BROWSER:
 		return parse_launch_browser(command, iter);
+	case STK_COMMAND_TYPE_OPEN_CHANNEL:
+		return parse_open_channel(command, iter);
 	case STK_COMMAND_TYPE_CLOSE_CHANNEL:
 		return parse_close_channel(command, iter);
 	case STK_COMMAND_TYPE_RECEIVE_DATA:
@@ -4752,12 +4871,35 @@ static gboolean build_dataobj_bearer_description(struct stk_tlv_builder *tlv,
 	const struct stk_bearer_description *bd = data;
 	unsigned char tag = STK_DATA_OBJECT_TYPE_BEARER_DESCRIPTION;
 
-	if (bd->type == 0x00)
+	if (bd->type != STK_BEARER_TYPE_GPRS_UTRAN)
 		return TRUE;
 
 	return stk_tlv_builder_open_container(tlv, cr, tag, FALSE) &&
 		stk_tlv_builder_append_byte(tlv, bd->type) &&
-		stk_tlv_builder_append_bytes(tlv, bd->pars, bd->len) &&
+		stk_tlv_builder_append_byte(tlv,
+			bd->gprs.precedence) &&
+		stk_tlv_builder_append_byte(tlv,
+			bd->gprs.delay) &&
+		stk_tlv_builder_append_byte(tlv,
+			bd->gprs.reliability) &&
+		stk_tlv_builder_append_byte(tlv,
+			bd->gprs.peak) &&
+		stk_tlv_builder_append_byte(tlv,
+			bd->gprs.mean) &&
+		stk_tlv_builder_append_byte(tlv,
+			bd->gprs.pdp_type) &&
+		stk_tlv_builder_close_container(tlv);
+}
+
+/* Described in TS 102.223 Section 8.53 */
+static gboolean build_dataobj_channel_data(struct stk_tlv_builder *tlv,
+						const void *data, gboolean cr)
+{
+	const struct stk_common_byte_array *cd = data;
+	unsigned char tag = STK_DATA_OBJECT_TYPE_CHANNEL_DATA;
+
+	return stk_tlv_builder_open_container(tlv, cr, tag, TRUE) &&
+		stk_tlv_builder_append_bytes(tlv, cd->array, cd->len) &&
 		stk_tlv_builder_close_container(tlv);
 }
 
@@ -4766,7 +4908,7 @@ static gboolean build_dataobj_channel_data_length(
 						struct stk_tlv_builder *tlv,
 						const void *data, gboolean cr)
 {
-	const unsigned int *length = data;
+	const unsigned short *length = data;
 	unsigned char tag = STK_DATA_OBJECT_TYPE_CHANNEL_DATA_LENGTH;
 
 	return stk_tlv_builder_open_container(tlv, cr, tag, FALSE) &&
@@ -4774,15 +4916,50 @@ static gboolean build_dataobj_channel_data_length(
 		stk_tlv_builder_close_container(tlv);
 }
 
+/* Described in TS 102.223 Section 8.55 */
+static gboolean build_dataobj_buffer_size(struct stk_tlv_builder *tlv,
+					const void *data, gboolean cr)
+{
+	const unsigned short *buf_size = data;
+	unsigned char tag = STK_DATA_OBJECT_TYPE_BUFFER_SIZE;
+
+	return stk_tlv_builder_open_container(tlv, cr, tag, FALSE) &&
+		stk_tlv_builder_append_short(tlv, *buf_size) &&
+		stk_tlv_builder_close_container(tlv);
+}
+
 /* Described in TS 102.223 Section 8.56 */
 static gboolean build_dataobj_channel_status(struct stk_tlv_builder *tlv,
 						const void *data, gboolean cr)
 {
+	const struct stk_channel *channel = data;
 	unsigned char tag = STK_DATA_OBJECT_TYPE_CHANNEL_STATUS;
+	unsigned char byte[2];
+
+	switch (channel->status) {
+	case STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED:
+	case STK_CHANNEL_TCP_IN_CLOSED_STATE:
+		byte[0] = channel->id;
+		byte[1] = 0x00;
+		break;
+	case STK_CHANNEL_PACKET_DATA_SERVICE_ACTIVATED:
+	case STK_CHANNEL_TCP_IN_ESTABLISHED_STATE:
+		byte[0] = channel->id | 0x80;
+		byte[1] = 0x00;
+		break;
+	case STK_CHANNEL_TCP_IN_LISTEN_STATE:
+		byte[0] = channel->id | 0x40;
+		byte[1] = 0x00;
+		break;
+	case STK_CHANNEL_LINK_DROPPED:
+		byte[0] = channel->id;
+		byte[1] = 0x05;
+		break;
+	}
 
 	return stk_tlv_builder_open_container(tlv, cr, tag, FALSE) &&
-		stk_tlv_builder_append_bytes(tlv, data, 2) &&
-		stk_tlv_builder_close_container(tlv);
+			stk_tlv_builder_append_bytes(tlv, byte, 2) &&
+			stk_tlv_builder_close_container(tlv);
 }
 
 /* Described in TS 102.223 Section 8.58 */
@@ -5454,6 +5631,67 @@ static gboolean build_local_info(struct stk_tlv_builder *builder,
 	return FALSE;
 }
 
+static gboolean build_open_channel(struct stk_tlv_builder *builder,
+					const struct stk_response *response)
+{
+	const struct stk_response_open_channel *open_channel =
+		&response->open_channel;
+
+	/* insert channel identifier only in case of success */
+	if (response->result.type == STK_RESULT_TYPE_SUCCESS) {
+		if (build_dataobj(builder, build_dataobj_channel_status,
+						0, &open_channel->channel,
+						NULL) != TRUE)
+			return FALSE;
+	}
+
+	return build_dataobj(builder,
+				build_dataobj_bearer_description,
+				0, &open_channel->bearer_desc,
+				build_dataobj_buffer_size,
+				0, &open_channel->buf_size,
+				NULL);
+}
+
+static gboolean build_receive_data(struct stk_tlv_builder *builder,
+					const struct stk_response *response)
+{
+	const struct stk_response_receive_data *receive_data =
+		&response->receive_data;
+
+	if (response->result.type == STK_RESULT_TYPE_SUCCESS) {
+		if (receive_data->rx_data.len) {
+			if (build_dataobj(builder, build_dataobj_channel_data,
+					DATAOBJ_FLAG_CR,
+					&response->receive_data.rx_data,
+					NULL) != TRUE)
+				return FALSE;
+		}
+
+		return build_dataobj(builder,
+					build_dataobj_channel_data_length,
+					DATAOBJ_FLAG_CR,
+					&response->receive_data.rx_remaining,
+					NULL);
+	}
+
+	return TRUE;
+}
+
+static gboolean build_send_data(struct stk_tlv_builder *builder,
+					const struct stk_response *response)
+{
+	if (response->result.type == STK_RESULT_TYPE_SUCCESS) {
+		return build_dataobj(builder,
+					build_dataobj_channel_data_length,
+					DATAOBJ_FLAG_CR,
+					&response->send_data.tx_avail,
+					NULL);
+	}
+
+	return TRUE;
+}
+
 const unsigned char *stk_pdu_from_response(const struct stk_response *response,
 						unsigned int *out_length)
 {
@@ -5582,6 +5820,7 @@ const unsigned char *stk_pdu_from_response(const struct stk_response *response,
 	case STK_COMMAND_TYPE_SEND_DTMF:
 	case STK_COMMAND_TYPE_LANGUAGE_NOTIFICATION:
 	case STK_COMMAND_TYPE_LAUNCH_BROWSER:
+	case STK_COMMAND_TYPE_CLOSE_CHANNEL:
 		break;
 	case STK_COMMAND_TYPE_SEND_USSD:
 		ok = build_dataobj(&builder,
@@ -5590,6 +5829,22 @@ const unsigned char *stk_pdu_from_response(const struct stk_response *response,
 					&response->send_ussd.text,
 					NULL);
 		break;
+	case STK_COMMAND_TYPE_OPEN_CHANNEL:
+		ok = build_open_channel(&builder, response);
+		break;
+	case STK_COMMAND_TYPE_RECEIVE_DATA:
+		ok = build_receive_data(&builder, response);
+		break;
+	case STK_COMMAND_TYPE_SEND_DATA:
+		ok = build_send_data(&builder, response);
+		break;
+	case STK_COMMAND_TYPE_GET_CHANNEL_STATUS:
+		ok = build_dataobj(&builder,
+					build_dataobj_channel_status,
+					DATAOBJ_FLAG_CR,
+					&response->channel_status.channel,
+					NULL);
+		break;
 	default:
 		return NULL;
 	};
@@ -5738,7 +5993,7 @@ static gboolean build_envelope_event_download(struct stk_tlv_builder *builder,
 		return build_dataobj(builder,
 					build_dataobj_channel_status,
 					DATAOBJ_FLAG_CR,
-					&evt->data_available.channel_status,
+					&evt->data_available.channel,
 					build_dataobj_channel_data_length,
 					DATAOBJ_FLAG_CR,
 					&evt->data_available.channel_data_len,
@@ -5747,7 +6002,7 @@ static gboolean build_envelope_event_download(struct stk_tlv_builder *builder,
 		return build_dataobj(builder,
 					build_dataobj_channel_status,
 					DATAOBJ_FLAG_CR,
-					&evt->channel_status.status,
+					&evt->channel_status.channel,
 					build_dataobj_bearer_description,
 					DATAOBJ_FLAG_CR,
 					&evt->channel_status.bearer_desc,
diff --git a/src/stkutil.h b/src/stkutil.h
index f2df23f..7dfb9a6 100644
--- a/src/stkutil.h
+++ b/src/stkutil.h
@@ -425,14 +425,19 @@ enum stk_browser_termination_cause {
 	STK_BROWSER_ERROR_TERMINATION =		0x01
 };
 
+/* Defined in TS 31.111 Section 8.52 */
 enum stk_bearer_type {
+	STK_BEARER_TYPE_CS =			0x01,
+	STK_BEARER_TYPE_GPRS_UTRAN =		0x02,
 	STK_BEARER_TYPE_DEFAULT =		0x03,
 	STK_BEARER_TYPE_INDEPENDENT =		0x04,
 	STK_BEARER_TYPE_BLUETOOTH =		0x05,
 	STK_BEARER_TYPE_IRDA =			0x06,
 	STK_BEARER_TYPE_RS232 =			0x07,
-	STK_BEARER_TYPE_PACKET_DATA_SERVICE =	0x08,
-	STK_BEARER_TYPE_I_WLAN =		0x0a,
+	STK_BEARER_TYPE_TIA_EIA_IS_820 =	0x08,
+	STK_BEARER_TYPE_UTRAN_WITH_EXT_PARAMS = 0x09,
+	STK_BEARER_TYPE_I_WLAN =		0x0A,
+	STK_BEARER_TYPE_EUTRAN_MAPPED_UTRAN =	0x0B,
 	STK_BEARER_TYPE_USB =			0x10
 };
 
@@ -587,6 +592,40 @@ enum stk_img_scheme {
 	STK_IMG_SCHEME_TRANSPARENCY =	0x22,
 };
 
+/* Defined in TS 102.223 Section 8.6 */
+enum stk_qualifier_open_channel {
+	STK_OPEN_CHANNEL_ON_DEMAND =			0x00,
+	STK_OPEN_CHANNEL_IMMEDIATE =			0x01,
+	STK_OPEN_CHANNEL_AUTOMATIC_RECONNECTION =	0x02,
+	STK_OPEN_CHANNEL_IMMEDIATE_IN_BACKGROUND_MODE =	0x04,
+};
+
+/* Defined in TS 102.223 Section 8.1 */
+enum stk_qualifier_send_data {
+	STK_SEND_DATA_STORE_DATA =	0x00,
+	STK_SEND_DATA_IMMEDIATELY =	0x01,
+};
+
+/* Defined in TS 102.223 Section 8.56 */
+enum stk_channel_status {
+	STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED = 0x00,
+	STK_CHANNEL_PACKET_DATA_SERVICE_ACTIVATED =	0x01,
+	STK_CHANNEL_TCP_IN_CLOSED_STATE =		0x02,
+	STK_CHANNEL_TCP_IN_LISTEN_STATE =		0x03,
+	STK_CHANNEL_TCP_IN_ESTABLISHED_STATE =		0x04,
+	STK_CHANNEL_LINK_DROPPED =			0x05,
+};
+
+/* Defined in TS 102.223 Section 8.59 */
+enum stk_transport_protocol_type {
+	STK_TRANSPORT_PROTOCOL_UDP_CLIENT_REMOTE =	0x01,
+	STK_TRANSPORT_PROTOCOL_TCP_CLIENT_REMOTE =	0x02,
+	STK_TRANSPORT_PROTOCOL_TCP_SERVER =		0x03,
+	STK_TRANSPORT_PROTOCOL_UDP_CLIENT_LOCAL =	0x04,
+	STK_TRANSPORT_PROTOCOL_TCP_CLIENT_LOCAL =	0x05,
+	STK_TRANSPORT_PROTOCOL_DIRECT =			0x06,
+};
+
 /* For data object that only has a byte array with undetermined length */
 struct stk_common_byte_array {
 	unsigned char *array;
@@ -849,16 +888,20 @@ struct stk_timing_advance {
 	unsigned char advance;
 };
 
-/*
- * According to 102.223 Section 8.52 the length of CTLV is 1 byte. This means
- * that the maximum size is 127 according to the rules of CTLVs. This size also
- * includes bearer type for 1 byte, so the maxmimum size of bearer parameters
- * is 126.
- */
+/* Bearer parameters for GPRS/UTRAN Packet Service/E-UTRAN */
+struct stk_gprs_bearer_parameters {
+	unsigned char precedence;
+	unsigned char delay;
+	unsigned char reliability;
+	unsigned char peak;
+	unsigned char mean;
+	unsigned char pdp_type;
+};
+
+/* Defined in TS 31.111 Section 8.52 */
 struct stk_bearer_description {
-	unsigned char type;
-	unsigned char pars[126];
-	unsigned int len;
+	enum stk_bearer_type type;
+	struct stk_gprs_bearer_parameters gprs;
 };
 
 /*
@@ -885,7 +928,7 @@ struct stk_other_address {
 
 /* Defined in TS 102.223 Section 8.59 */
 struct stk_uicc_te_interface {
-	unsigned char protocol;
+	enum stk_transport_protocol_type protocol;
 	unsigned short port;
 };
 
@@ -950,15 +993,6 @@ struct stk_remote_entity_address {
 };
 
 /*
- * According to 102.223 Section 8.70 the length of CTLV is 1 byte. This means
- * that the maximum size is 127 according to the rules of CTLVs.
- */
-struct stk_network_access_name {
-	unsigned char name[127];
-	unsigned char len;
-};
-
-/*
  * According to 102.223 Section 8.72 the length of text attribute CTLV is 1
  * byte.  This means that the maximum size is 127 according to the rules
  * of CTLVs.  Empty attribute options will have len of 0.
@@ -1250,6 +1284,21 @@ struct stk_command_launch_browser {
 	char *text_passwd;
 };
 
+struct stk_command_open_channel {
+	char *alpha_id;
+	struct stk_icon_id icon_id;
+	struct stk_bearer_description bearer_desc;
+	unsigned short buf_size;
+	char *apn;
+	struct stk_other_address local_addr;
+	char *text_usr;
+	char *text_passwd;
+	struct stk_uicc_te_interface uti;
+	struct stk_other_address data_dest_addr;
+	struct stk_text_attribute text_attr;
+	struct stk_frame_id frame_id;
+};
+
 struct stk_command_close_channel {
 	char *alpha_id;
 	struct stk_icon_id icon_id;
@@ -1368,6 +1417,7 @@ struct stk_command {
 		struct stk_command_send_dtmf send_dtmf;
 		struct stk_command_language_notification language_notification;
 		struct stk_command_launch_browser launch_browser;
+		struct stk_command_open_channel open_channel;
 		struct stk_command_close_channel close_channel;
 		struct stk_command_receive_data receive_data;
 		struct stk_command_send_data send_data;
@@ -1406,6 +1456,19 @@ struct stk_ussd_text {
 	int len;
 };
 
+struct stk_channel {
+	unsigned char id;
+	enum stk_channel_status status;
+};
+
+struct stk_channel_data {
+	struct stk_common_byte_array data;
+	union {
+		unsigned short rx_remaining;
+		unsigned short tx_avail;
+	};
+};
+
 struct stk_response_get_inkey {
 	struct stk_answer_text text;
 	struct stk_duration duration;
@@ -1481,6 +1544,25 @@ struct stk_response_send_ussd {
 	struct stk_ussd_text text;
 };
 
+struct stk_response_open_channel {
+	struct stk_channel channel;
+	struct stk_bearer_description bearer_desc;
+	unsigned short buf_size;
+};
+
+struct stk_response_receive_data {
+	struct stk_common_byte_array rx_data;
+	unsigned char rx_remaining;
+};
+
+struct stk_response_send_data {
+	unsigned char tx_avail;
+};
+
+struct stk_response_channel_status {
+	struct stk_channel channel;
+};
+
 struct stk_response {
 	unsigned char number;
 	unsigned char type;
@@ -1511,6 +1593,10 @@ struct stk_response {
 		struct stk_response_generic language_notification;
 		struct stk_response_generic launch_browser;
 		struct stk_response_send_ussd send_ussd;
+		struct stk_response_open_channel open_channel;
+		struct stk_response_receive_data receive_data;
+		struct stk_response_send_data send_data;
+		struct stk_response_channel_status channel_status;
 	};
 
 	void (*destructor)(struct stk_response *response);
@@ -1596,11 +1682,11 @@ struct stk_envelope_event_download {
 			enum stk_browser_termination_cause cause;
 		} browser_termination;
 		struct {
-			unsigned char channel_status[2];
+			struct stk_channel channel;
 			unsigned int channel_data_len;
 		} data_available;
 		struct {
-			unsigned char status[2];
+			struct stk_channel channel;
 			struct stk_bearer_description bearer_desc;
 			struct stk_other_address address;
 		} channel_status;
-- 
1.7.1

Re: [PATCH v2 1/2] stkutil: Complete the TLV parsing/builder to support BIP commands

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 5142 bytes --]

Hi Philippe,

On 03/30/2011 04:41 PM, Philippe Nunes wrote:
> ---
>  src/stkutil.c |  287 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  src/stkutil.h |  132 ++++++++++++++++++++++-----
>  2 files changed, 380 insertions(+), 39 deletions(-)

<snip>

> 
> @@ -1604,16 +1624,48 @@ static gboolean parse_dataobj_esn(struct comprehension_tlv_iter *iter,
>  static gboolean parse_dataobj_network_access_name(
>  		struct comprehension_tlv_iter *iter, void *user)
>  {
> -	struct stk_network_access_name *nan = user;
> +	char **apn = user;
>  	const unsigned char *data;
>  	unsigned int len = comprehension_tlv_iter_get_length(iter);
> +	unsigned char label_size;
> +	unsigned char offset = 0;
> +	char decoded_apn[100];
>  
> -	if (len == 0)
> +	if (len == 0 || len > 100)
>  		return FALSE;
>  
>  	data = comprehension_tlv_iter_get_data(iter);
> -	nan->len = len;
> -	memcpy(nan->name, data, len);
> +	/*
> +	 * As specified in TS 23 003 Section 9
> +	 * The APN consists of one or more labels. Each label is coded as
> +	 * a one octet length field followed by that number of octets coded
> +	 * as 8 bit ASCII characters
> +	 */
> +
> +	while (len) {
> +		label_size = *data;
> +
> +		if (label_size > (len-1))
> +			return FALSE;
> +
> +		memcpy(decoded_apn + offset, data + 1, label_size);
> +
> +		data += label_size + 1;
> +		offset += label_size;
> +
> +		len -= label_size + 1;

This really belongs with the data/offset increments

> +		if (len) {
> +			decoded_apn[offset] = '.';
> +			offset += 1;

This might be better written as:

if (len > 0)
	decoded_apn[offset++] = '.';

> +		}
> +	}
> +
> +	*apn = g_try_malloc(offset + 1);
> +	if (*apn == NULL)
> +		return FALSE;
> +
> +	memcpy(*apn, decoded_apn, offset);
> +	(*apn)[offset] = '\0';

*apn = g_strdup(decoded_apn);

is really just fine ;)

>  
>  	return TRUE;
>  }
> @@ -3274,7 +3326,68 @@ static enum stk_command_parse_result parse_launch_browser(
>  				STK_DATA_OBJECT_TYPE_INVALID);
>  }
>  
> -/* TODO: parse_open_channel */
> +static void destroy_open_channel(struct stk_command *command)
> +{
> +	g_free(command->open_channel.alpha_id);
> +	g_free(command->open_channel.text_usr);
> +	g_free(command->open_channel.text_passwd);
> +}

==9744== 40 bytes in 4 blocks are definitely lost in loss record 1,825
of 1,925
==9744==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==9744==    by 0x41B868: parse_dataobj_network_access_name (stkutil.c:1663)
==9744==    by 0x41AF95: parse_dataobj (stkutil.c:2415)
==9744==    by 0x41CC1B: stk_command_new_from_pdu (stkutil.c:3358)
==9744==    by 0x40A953: test_open_channel (test-stkutil.c:17257)
==9744==    by 0x4E922E2: g_test_run_suite_internal (gtestutils.c:1138)
==9744==    by 0x4E92455: g_test_run_suite_internal (gtestutils.c:1197)
==9744==    by 0x4E9273A: g_test_run_suite (gtestutils.c:1246)
==9744==    by 0x406C45: main (test-stkutil.c:26130)

<snip>

> +static gboolean build_receive_data(struct stk_tlv_builder *builder,
> +					const struct stk_response *response)
> +{
> +	const struct stk_response_receive_data *receive_data =
> +		&response->receive_data;
> +
> +	if (response->result.type == STK_RESULT_TYPE_SUCCESS) {
> +		if (receive_data->rx_data.len) {
> +			if (build_dataobj(builder, build_dataobj_channel_data,
> +					DATAOBJ_FLAG_CR,
> +					&response->receive_data.rx_data,
> +					NULL) != TRUE)
> +				return FALSE;
> +		}
> +
> +		return build_dataobj(builder,
> +					build_dataobj_channel_data_length,
> +					DATAOBJ_FLAG_CR,
> +					&response->receive_data.rx_remaining,
> +					NULL);
> +	}

Avoid nesting whenever possible.  This really might be better written like:

if (response->result.type != STK_RESULT_TYPE_SUCCESS)
	return TRUE;

if (...) {
}

return build_dataobj(...)

> +
> +	return TRUE;
> +}
> +
> +static gboolean build_send_data(struct stk_tlv_builder *builder,
> +					const struct stk_response *response)
> +{
> +	if (response->result.type == STK_RESULT_TYPE_SUCCESS) {
> +		return build_dataobj(builder,
> +					build_dataobj_channel_data_length,
> +					DATAOBJ_FLAG_CR,
> +					&response->send_data.tx_avail,
> +					NULL);
> +	}
> +

Same comment as above

> +	return TRUE;
> +}
> +
>  const unsigned char *stk_pdu_from_response(const struct stk_response *response,
>  						unsigned int *out_length)
>  {
> +/* Defined in TS 102.223 Section 8.1 */

^^^

This really doesn't sounds right for the Section

> +enum stk_qualifier_send_data {
> +	STK_SEND_DATA_STORE_DATA =	0x00,
> +	STK_SEND_DATA_IMMEDIATELY =	0x01,
> +};
> +
> +/* Defined in TS 102.223 Section 8.56 */
> +enum stk_channel_status {
> +	STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED = 0x00,
> +	STK_CHANNEL_PACKET_DATA_SERVICE_ACTIVATED =	0x01,
> +	STK_CHANNEL_TCP_IN_CLOSED_STATE =		0x02,
> +	STK_CHANNEL_TCP_IN_LISTEN_STATE =		0x03,
> +	STK_CHANNEL_TCP_IN_ESTABLISHED_STATE =		0x04,
> +	STK_CHANNEL_LINK_DROPPED =			0x05,
> +};
> +

You're playing serious magic here, but fair enough.

Regards,
-Denis

Re: [PATCH v2 1/2] stkutil: Complete the TLV parsing/builder to support BIP commands

[Philippe Nunes]

[-- Attachment #1: Type: text/plain, Size: 6193 bytes --]

Hi Denis,

On 03/31/2011 03:26 AM, Denis Kenzior wrote:
> Hi Philippe,
>
> On 03/30/2011 04:41 PM, Philippe Nunes wrote:
>> ---
>>   src/stkutil.c |  287 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   src/stkutil.h |  132 ++++++++++++++++++++++-----
>>   2 files changed, 380 insertions(+), 39 deletions(-)
>
> <snip>
>
>>
>> @@ -1604,16 +1624,48 @@ static gboolean parse_dataobj_esn(struct comprehension_tlv_iter *iter,
>>   static gboolean parse_dataobj_network_access_name(
>>   		struct comprehension_tlv_iter *iter, void *user)
>>   {
>> -	struct stk_network_access_name *nan = user;
>> +	char **apn = user;
>>   	const unsigned char *data;
>>   	unsigned int len = comprehension_tlv_iter_get_length(iter);
>> +	unsigned char label_size;
>> +	unsigned char offset = 0;
>> +	char decoded_apn[100];
>>
>> -	if (len == 0)
>> +	if (len == 0 || len>  100)
>>   		return FALSE;
>>
>>   	data = comprehension_tlv_iter_get_data(iter);
>> -	nan->len = len;
>> -	memcpy(nan->name, data, len);
>> +	/*
>> +	 * As specified in TS 23 003 Section 9
>> +	 * The APN consists of one or more labels. Each label is coded as
>> +	 * a one octet length field followed by that number of octets coded
>> +	 * as 8 bit ASCII characters
>> +	 */
>> +
>> +	while (len) {
>> +		label_size = *data;
>> +
>> +		if (label_size>  (len-1))
>> +			return FALSE;
>> +
>> +		memcpy(decoded_apn + offset, data + 1, label_size);
>> +
>> +		data += label_size + 1;
>> +		offset += label_size;
>> +
>> +		len -= label_size + 1;
>
> This really belongs with the data/offset increments
>
>> +		if (len) {
>> +			decoded_apn[offset] = '.';
>> +			offset += 1;
>
> This might be better written as:
>
> if (len>  0)
> 	decoded_apn[offset++] = '.';
>
>> +		}
>> +	}
>> +
>> +	*apn = g_try_malloc(offset + 1);
>> +	if (*apn == NULL)
>> +		return FALSE;
>> +
>> +	memcpy(*apn, decoded_apn, offset);
>> +	(*apn)[offset] = '\0';
>
> *apn = g_strdup(decoded_apn);
>
> is really just fine ;)
>
>>

Ok, I'm just going to use g_strndup to get the string null terminated.


>>   	return TRUE;
>>   }
>> @@ -3274,7 +3326,68 @@ static enum stk_command_parse_result parse_launch_browser(
>>   				STK_DATA_OBJECT_TYPE_INVALID);
>>   }
>>
>> -/* TODO: parse_open_channel */
>> +static void destroy_open_channel(struct stk_command *command)
>> +{
>> +	g_free(command->open_channel.alpha_id);
>> +	g_free(command->open_channel.text_usr);
>> +	g_free(command->open_channel.text_passwd);
>> +}
>
> ==9744== 40 bytes in 4 blocks are definitely lost in loss record 1,825
> of 1,925
> ==9744==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
> ==9744==    by 0x41B868: parse_dataobj_network_access_name (stkutil.c:1663)
> ==9744==    by 0x41AF95: parse_dataobj (stkutil.c:2415)
> ==9744==    by 0x41CC1B: stk_command_new_from_pdu (stkutil.c:3358)
> ==9744==    by 0x40A953: test_open_channel (test-stkutil.c:17257)
> ==9744==    by 0x4E922E2: g_test_run_suite_internal (gtestutils.c:1138)
> ==9744==    by 0x4E92455: g_test_run_suite_internal (gtestutils.c:1197)
> ==9744==    by 0x4E9273A: g_test_run_suite (gtestutils.c:1246)
> ==9744==    by 0x406C45: main (test-stkutil.c:26130)
>


Right, I missed to free the apn string.



> <snip>
>
>> +static gboolean build_receive_data(struct stk_tlv_builder *builder,
>> +					const struct stk_response *response)
>> +{
>> +	const struct stk_response_receive_data *receive_data =
>> +		&response->receive_data;
>> +
>> +	if (response->result.type == STK_RESULT_TYPE_SUCCESS) {
>> +		if (receive_data->rx_data.len) {
>> +			if (build_dataobj(builder, build_dataobj_channel_data,
>> +					DATAOBJ_FLAG_CR,
>> +					&response->receive_data.rx_data,
>> +					NULL) != TRUE)
>> +				return FALSE;
>> +		}
>> +
>> +		return build_dataobj(builder,
>> +					build_dataobj_channel_data_length,
>> +					DATAOBJ_FLAG_CR,
>> +					&response->receive_data.rx_remaining,
>> +					NULL);
>> +	}
>
> Avoid nesting whenever possible.  This really might be better written like:
>
> if (response->result.type != STK_RESULT_TYPE_SUCCESS)
> 	return TRUE;
>
> if (...) {
> }
>
> return build_dataobj(...)
>
>> +
>> +	return TRUE;
>> +}
>> +
>> +static gboolean build_send_data(struct stk_tlv_builder *builder,
>> +					const struct stk_response *response)
>> +{
>> +	if (response->result.type == STK_RESULT_TYPE_SUCCESS) {
>> +		return build_dataobj(builder,
>> +					build_dataobj_channel_data_length,
>> +					DATAOBJ_FLAG_CR,
>> +					&response->send_data.tx_avail,
>> +					NULL);
>> +	}
>> +
>
> Same comment as above
>

Ok, I try to keep this rule in mind.


>> +	return TRUE;
>> +}
>> +
>>   const unsigned char *stk_pdu_from_response(const struct stk_response *response,
>>   						unsigned int *out_length)
>>   {
>> +/* Defined in TS 102.223 Section 8.1 */
>
> ^^^
>
> This really doesn't sounds right for the Section
>

Yes, corrected to section 8.6 (command qualifier)

>> +enum stk_qualifier_send_data {
>> +	STK_SEND_DATA_STORE_DATA =	0x00,
>> +	STK_SEND_DATA_IMMEDIATELY =	0x01,
>> +};
>> +
>> +/* Defined in TS 102.223 Section 8.56 */
>> +enum stk_channel_status {
>> +	STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED = 0x00,
>> +	STK_CHANNEL_PACKET_DATA_SERVICE_ACTIVATED =	0x01,
>> +	STK_CHANNEL_TCP_IN_CLOSED_STATE =		0x02,
>> +	STK_CHANNEL_TCP_IN_LISTEN_STATE =		0x03,
>> +	STK_CHANNEL_TCP_IN_ESTABLISHED_STATE =		0x04,
>> +	STK_CHANNEL_LINK_DROPPED =			0x05,
>> +};
>> +
>
> You're playing serious magic here, but fair enough.


Here, I'm introducing these enums to list only the different channel 
status but they can't be used indeed as direct values for the channel 
status TLV parsing/building.

Note that for the section:

enum stk_qualifier_open_channel {
	STK_OPEN_CHANNEL_ON_DEMAND =			0x00,
	STK_OPEN_CHANNEL_IMMEDIATE =			0x01,
	STK_OPEN_CHANNEL_AUTOMATIC_RECONNECTION =	0x02,
	STK_OPEN_CHANNEL_IMMEDIATE_IN_BACKGROUND_MODE =	0x04,
};

The purpose of these enums is to use them as bit masks. Maybe #defines 
could be more appropriate?

Regards,

Philippe.

Re: [PATCH v2 1/2] stkutil: Complete the TLV parsing/builder to support BIP commands

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

Hi Philippe,

>>> +        }
>>> +    }
>>> +
>>> +    *apn = g_try_malloc(offset + 1);
>>> +    if (*apn == NULL)
>>> +        return FALSE;
>>> +
>>> +    memcpy(*apn, decoded_apn, offset);
>>> +    (*apn)[offset] = '\0';
>>
>> *apn = g_strdup(decoded_apn);
>>
>> is really just fine ;)
>>
>>>
> 
> Ok, I'm just going to use g_strndup to get the string null terminated.
> 
> 

Might be easier to null terminate decoded_apn and use g_strdup.  That
might be a bit easier to read (and a more common variation) then trying
to use g_strndup.

>>> +/* Defined in TS 102.223 Section 8.56 */
>>> +enum stk_channel_status {
>>> +    STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED = 0x00,
>>> +    STK_CHANNEL_PACKET_DATA_SERVICE_ACTIVATED =    0x01,
>>> +    STK_CHANNEL_TCP_IN_CLOSED_STATE =        0x02,
>>> +    STK_CHANNEL_TCP_IN_LISTEN_STATE =        0x03,
>>> +    STK_CHANNEL_TCP_IN_ESTABLISHED_STATE =        0x04,
>>> +    STK_CHANNEL_LINK_DROPPED =            0x05,
>>> +};
>>> +
>>
>> You're playing serious magic here, but fair enough.
> 
> 
> Here, I'm introducing these enums to list only the different channel
> status but they can't be used indeed as direct values for the channel
> status TLV parsing/building.
> 
> Note that for the section:
> 
> enum stk_qualifier_open_channel {
>     STK_OPEN_CHANNEL_ON_DEMAND =            0x00,
>     STK_OPEN_CHANNEL_IMMEDIATE =            0x01,
>     STK_OPEN_CHANNEL_AUTOMATIC_RECONNECTION =    0x02,
>     STK_OPEN_CHANNEL_IMMEDIATE_IN_BACKGROUND_MODE =    0x04,
> };
> 
> The purpose of these enums is to use them as bit masks. Maybe #defines
> could be more appropriate?

I'd almost suggest booleans or 3 separate enums instead.  However, it is
fine to use enums in such a manner (though I don't see the point of 0x00
value).  It might be better to indicate they're flags / bits in the enum
name.  e.g. STK_OPEN_CHANNEL_FLAG_IMMEDIATE,
STK_OEPN_CHANNEL_FLAG_BACKGROUND, etc.

Regards,
-Denis