Fix the structure of stk_subaddress

 src/simutil.c |    2 +-
 src/simutil.h |    1 +
 src/stkutil.c |   40 +++++++++++++++++++++++++++++++++++++---
 src/stkutil.h |    9 +++++++++
 4 files changed, 48 insertions(+), 4 deletions(-)

src/simutil.c +1 -1

index d9383b7..65ffa36 100644
@@ -538,7 +538,7 @@ static char *sim_network_name_parse(const unsigned char *buffer, int length,
 	return ret;
 }
 
-static void parse_mcc_mnc(const guint8 *bcd, char *mcc, char *mnc)
+void parse_mcc_mnc(const guint8 *bcd, char *mcc, char *mnc)
 {
 	static const char digit_lut[] = "0123456789*#abd\0";
 	guint8 digit;

src/simutil.h +1 -0

index 043c21f..09964a8 100644
@@ -181,6 +181,7 @@ const struct sim_eons_operator_info *sim_eons_lookup(struct sim_eons *eons,
 						const char *mnc);
 void sim_eons_free(struct sim_eons *eons);
 
+void parse_mcc_mnc(const guint8 *bcd, char *mcc, char *mnc);
 struct sim_spdi *sim_spdi_new(const guint8 *tlv, int length);
 gboolean sim_spdi_lookup(struct sim_spdi *spdi,
 				const char *mcc, const char *mnc);

src/stkutil.c +0 -0

index 9f3bc0b..9fa7705 100644

---
 src/stkutil.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 src/stkutil.h |    6 ++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

src/stkutil.c +23 -0

index ceba2d5..9f3bc0b 100644
@@ -406,6 +406,46 @@ static gboolean parse_dataobj_tone(struct comprehension_tlv_iter *iter,
 	return TRUE;
 }
 
+/* Defined in TS 102.223 Section 8.18 */
+static gboolean parse_dataobj_file_list(struct comprehension_tlv_iter *iter,
+						void *user)
+{
+	GSList **fl = user;
+	const unsigned char *data;
+	unsigned int len;
+	unsigned int i;
+	unsigned int start = 1;
+	struct stk_file *sf;
+
+	if (comprehension_tlv_iter_get_tag(iter) !=
+			STK_DATA_OBJECT_TYPE_FILE_LIST)
+		return FALSE;
+
+	len = comprehension_tlv_iter_get_length(iter);
+	if (len < 5)
+		return FALSE;
+
+	data = comprehension_tlv_iter_get_data(iter);
+
+	if (data[start] != 0x3f)
+		return FALSE;

---
 src/stkutil.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 src/stkutil.h |    6 ++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

src/stkutil.c +24 -0

index ceba2d5..9f3bc0b 100644
@@ -406,6 +406,46 @@ static gboolean parse_dataobj_tone(struct
 comprehension_tlv_iter *iter, return TRUE;
 }
 
+/* Defined in TS 102.223 Section 8.18 */
+static gboolean parse_dataobj_file_list(struct comprehension_tlv_iter
 *iter, +						void *user)
+{
+	GSList **fl = user;
+	const unsigned char *data;
+	unsigned int len;
+	unsigned int i;
+	unsigned int start = 1;
+	struct stk_file *sf;
+
+	if (comprehension_tlv_iter_get_tag(iter) !=
+			STK_DATA_OBJECT_TYPE_FILE_LIST)
+		return FALSE;
+
+	len = comprehension_tlv_iter_get_length(iter);
+	if (len < 5)
+		return FALSE;
+
+	data = comprehension_tlv_iter_get_data(iter);
+
+	if (data[start] != 0x3f)
+		return FALSE;
+
+	for (i = start + 4; i <= len; i += 2) {

src/stkutil.c +7 -0

+	*fl = g_slist_reverse(*fl);
+
+	return TRUE;
+}
+
 /* Defined in TS 102.223 Section 8.31 */
 static gboolean parse_dataobj_icon_id(struct comprehension_tlv_iter *iter,
 					void *user)

@@ -523,6 +563,8 @@ static dataobj_handler handler_for_type(enum
 stk_data_object_type type) return parse_dataobj_text;
 	case STK_DATA_OBJECT_TYPE_TONE:
 		return parse_dataobj_tone;
+	case STK_DATA_OBJECT_TYPE_FILE_LIST:
+		return parse_dataobj_file_list;
 	case STK_DATA_OBJECT_TYPE_ICON_ID:
 		return parse_dataobj_icon_id;
 	case STK_DATA_OBJECT_TYPE_IMMEDIATE_RESPONSE:

src/stkutil.h +6 -0

index b408f38..5b1a44b 100644
@@ -345,6 +345,12 @@ struct stk_result {
 	unsigned char *additional;
 };
 
+/* Define the struct of single file in TS102.223 Section 8.18 */
+struct stk_file {
+	unsigned char *file;
+	unsigned int 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

 src/stkutil.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

src/stkutil.c +1 -0

index 787f7eb..ceba2d5 100644
@@ -190,6 +190,7 @@ static gboolean parse_dataobj_item(struct comprehension_tlv_iter *iter,
 	struct stk_item *item = user;
 	const unsigned char *data;
 	unsigned int len;
+	char *utf8;
 
 	if (comprehension_tlv_iter_get_tag(iter) !=
 			STK_DATA_OBJECT_TYPE_ITEM)

src/stkutil.c static gboolean parse_dataobj_item(struct comprehension_tlv_iter *iter, · line 215

Quoted 4 lines

src/stkutil.c +4 -0

+	if (utf8 == NULL)
+		return FALSE;
+
+	item->text = utf8;

Why bother with utf8 variable? Just do

	if (item->text == NULL)
		return FALSE;

Quoted 7 lines

src/stkutil.c static gboolean parse_dataobj_item(struct comprehension_tlv_iter *iter, · line 215

Quoted 4 lines

src/stkutil.c +4 -0

+	if (utf8 == NULL)
+		return FALSE;
+
+	item->text = utf8;

Why bother with utf8 variable? Just do

Quoted 3 lines

	if (item->text == NULL)
		return FALSE;

I actually find this acceptable because 'item' is a return structure, so we should avoid modifying it in case of an error.

-----Original Message-----
From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf Of Denis Kenzior
Sent: Wednesday, March 17, 2010 4:30 AM
To: ofono(a)ofono.org
Subject: Re: [PATCH 2/4] Handle the conversion failure when parsing item

Hi Yang,

src/stkutil.c static gboolean parse_dataobj_item(struct comprehension_tlv_iter *iter, · line 210

Quoted 2 lines

src/stkutil.c +1 -1

-	item->text = sim_string_to_utf8(data+1, len-1);
+	utf8 = sim_string_to_utf8(data+1, len-1);

I applied the patch and fixed up the arithmetic operator style issue. Please make sure you follow this convention from now on.

Regards,
-Denis
_______________________________________________
ofono mailing list
ofono(a)ofono.org
http://lists.ofono.org/listinfo/ofono

Quoted 5 lines

src/stkutil.c static gboolean parse_dataobj_item(struct comprehension_tlv_iter *iter, · line 210

Quoted 2 lines

src/stkutil.c +1 -1

-	item->text = sim_string_to_utf8(data+1, len-1);
+	utf8 = sim_string_to_utf8(data+1, len-1);

I applied the patch and fixed up the arithmetic operator style issue. Please make sure you follow this convention from now on.

Thank you, and I will pay attention to this in future.

I'm always wondering what's the complete coding style we should follow. Can we have some document to describe them? I have collected some of them:
1. Check the patch using checkpatch.pl (checkpatch.pl --no-tree patch_name). In theory, you need to clean up all the warnings and errors except one "ERROR: Missing Signed-off-by: line(s)". However, sometimes, the warning of exceeding maximum characters for a line (80 characters) can be ignored.
2. There should be a blank line before if statement, unless it is nested and not preceded by an expression or variable declaration.
3. There should be space before and after operator.
4. It's better not to have a complicated statement for if. You may judge its contrary condition and return | break | continue ASAP.
5. Better to use abbreviation, rather than full name, to name a variable, function, struct, etc.
Any others?

---
 src/stkutil.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)