[PATCH 0/3] html text attribute patches

[PATCH 0/3] html text attribute patches

[Kristen Carlson Accardi]

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

Modified stkutil per last review.  Added unit tests for Display Text
tests.  I will add the rest of the unit tests after this patch series
is reviewed.

Kristen Carlson Accardi (3):
  stkutil: display text attributes as html
  test-stkutil: add unit test for html text attributes
  test-stkutil: add html attribute test for Display Text tests

 src/stkutil.c       |  190 ++++++++++++++++++++++++++++++++++
 src/stkutil.h       |   23 ++++
 unit/test-stkutil.c |  280 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 493 insertions(+), 0 deletions(-)

[PATCH 1/3] stkutil: display text attributes as html

[Kristen Carlson Accardi]

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

---
 src/stkutil.c |  190 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkutil.h |   23 +++++++
 2 files changed, 213 insertions(+), 0 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 6f072e7..843e06b 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -5819,3 +5819,193 @@ const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
 
 	return pdu;
 }
+
+static const char *html_colors[] = {
+	"#000000", /* Black */
+	"#808080", /* Dark Grey */
+	"#C11B17", /* Dark Red */
+	"#FBB117", /* Dark Yellow */
+	"#347235", /* Dark Green */
+	"#307D7E", /* Dark Cyan */
+	"#0000A0", /* Dark Blue */
+	"#C031C7", /* Dark Magenta */
+	"#C0C0C0", /* Grey */
+	"#FFFFFF", /* White */
+	"#FF0000", /* Bright Red */
+	"#FFFF00", /* Bright Yellow */
+	"#00FF00", /* Bright Green */
+	"#00FFFF", /* Bright Cyan */
+	"#0000FF", /* Bright Blue */
+	"#FF00FF", /* Bright Magenta */
+};
+
+static void end_format(GString *string, guint8 code)
+{
+	g_string_append(string, "</span>");
+
+	if ((code & STK_TEXT_FORMAT_ALIGN_MASK) != STK_TEXT_FORMAT_NO_ALIGN)
+		g_string_append(string, "</div>");
+}
+
+static void start_format(GString *string, guint8 code, guint8 color)
+{
+	guint8 align = code & STK_TEXT_FORMAT_ALIGN_MASK;
+	guint8 font = code & STK_TEXT_FORMAT_FONT_MASK;
+	guint8 style = code & STK_TEXT_FORMAT_STYLE_MASK;
+	int fg = color & 0x0f;
+	int bg = (color >> 4) & 0x0f;
+
+	/* align formatting applies to a block of test */
+	if (align != STK_TEXT_FORMAT_NO_ALIGN)
+		g_string_append(string, "<div style=\"");
+
+	switch (align) {
+	case STK_TEXT_FORMAT_RIGHT_ALIGN:
+		g_string_append(string, "text-align: right;\">");
+		break;
+	case STK_TEXT_FORMAT_CENTER_ALIGN:
+		g_string_append(string, "text-align: center;\">");
+		break;
+	case STK_TEXT_FORMAT_LEFT_ALIGN:
+		g_string_append(string, "text-align: left;\">");
+		break;
+	}
+
+	/* font, style, and color are inline */
+	g_string_append(string, "<span style=\"");
+
+	switch (font) {
+	case STK_TEXT_FORMAT_FONT_SIZE_LARGE:
+		g_string_append(string, "font-size: big;");
+		break;
+	case STK_TEXT_FORMAT_FONT_SIZE_SMALL:
+		g_string_append(string, "font-size: small;");
+		break;
+	}
+
+	switch (style) {
+	case STK_TEXT_FORMAT_STYLE_BOLD:
+		g_string_append(string, "font-weight: bold;");
+		break;
+	case STK_TEXT_FORMAT_STYLE_ITALIC:
+		g_string_append(string, "font-style: italic;");
+		break;
+	case STK_TEXT_FORMAT_STYLE_UNDERLINED:
+		g_string_append(string, "text-decoration: underline;");
+		break;
+	case STK_TEXT_FORMAT_STYLE_STRIKETHROUGH:
+		g_string_append(string, "text-decoration: line-through;");
+		break;
+	}
+
+	/* add any color */
+	if (fg)
+		g_string_append_printf(string, "color: %s;", html_colors[fg]);
+	if (bg)
+		g_string_append_printf(string, "background-color: %s;",
+						html_colors[bg]);
+	g_string_append(string, "\">");
+}
+
+char *stk_text_to_html(char *text, int text_len,
+				const unsigned char *attrs, int attrs_len)
+{
+	GString *string = g_string_sized_new(text_len + 1);
+	int formats[257];  /* maximum number of chars in text + 1 */
+	int pos = 0, i, j, attr, prev_attr;
+	guint8 start, end, code, color, len, align;
+
+	/* we will need formatting at the position beyond the last char */
+	for (i = 0; i <= text_len; i++)
+		formats[i] = STK_TEXT_FORMAT_INIT;
+
+	i = 0;
+
+	while (i < attrs_len) {
+		start = attrs[i++];
+		len = attrs[i++];
+		code = attrs[i++];
+
+		if (i < attrs_len)
+			color = attrs[i++];
+		else
+			color = 0;
+
+		if (len == 0)
+			end = text_len;
+		else
+			end = start + len;
+
+		/* sanity check values */
+		if (start > end || end > text_len)
+			continue;
+
+		/*
+		 * if the alignment is the same as either the default
+		 * or the last alignment used, don't set any alignment
+		 * value.
+		 */
+		if (start == 0)
+			align = STK_DEFAULT_TEXT_ALIGNMENT;
+		else {
+			align = (formats[start -1] & 0xFF) &
+					STK_TEXT_FORMAT_ALIGN_MASK;
+			if (align == STK_TEXT_FORMAT_NO_ALIGN)
+				align = STK_DEFAULT_TEXT_ALIGNMENT;
+		}
+
+		if ((code & STK_TEXT_FORMAT_ALIGN_MASK) == align)
+			code |= STK_TEXT_FORMAT_NO_ALIGN;
+
+		attr = code | (color << 8);
+
+		for (j = start; j < end; j++)
+			formats[j] = attr;
+	}
+
+	prev_attr = STK_TEXT_FORMAT_INIT;
+
+	while (pos <= text_len) {
+		attr = formats[pos];
+		if (attr != prev_attr) {
+			if (prev_attr != STK_TEXT_FORMAT_INIT)
+				end_format(string, prev_attr & 0xFF);
+
+			if (attr != STK_TEXT_FORMAT_INIT)
+				start_format(string, attr & 0xFF,
+							(attr >> 8) & 0xFF);
+
+			prev_attr = attr;
+		}
+
+		if (pos == text_len)
+			break;
+
+		switch (text[pos]) {
+		case '\n':
+			g_string_append(string, "<br/>");
+			break;
+		case '\r':
+		{
+			g_string_append(string, "<br/>");
+			if ((pos + 1 < text_len) && (text[pos + 1] == '\n'))
+				pos++;
+		}
+		case '<':
+			g_string_append(string, "&lt;");
+			break;
+		case '>':
+			g_string_append(string, "&gt;");
+			break;
+		case '&':
+			g_string_append(string, "&amp;");
+			break;
+		default:
+			g_string_append_c(string, text[pos]);
+		}
+		pos++;
+	}
+
+	/* return characters from string. Caller must free char data */
+	return g_string_free(string, FALSE);
+}
diff --git a/src/stkutil.h b/src/stkutil.h
index ca4817e..5b3a679 100644
--- a/src/stkutil.h
+++ b/src/stkutil.h
@@ -1635,6 +1635,27 @@ struct stk_envelope {
 	};
 };
 
+#define STK_TEXT_FORMAT_ALIGN_MASK 0x03
+#define STK_TEXT_FORMAT_FONT_MASK 0x0C
+#define STK_TEXT_FORMAT_STYLE_MASK 0xF0
+#define STK_DEFAULT_TEXT_ALIGNMENT 0x00
+#define STK_TEXT_FORMAT_INIT -1
+
+/* Defined in ETSI 123 40 9.2.3.24.10.1.1 */
+enum stk_text_format_code {
+	STK_TEXT_FORMAT_LEFT_ALIGN = 0x00,
+	STK_TEXT_FORMAT_CENTER_ALIGN = 0x01,
+	STK_TEXT_FORMAT_RIGHT_ALIGN = 0x02,
+	STK_TEXT_FORMAT_NO_ALIGN = 0x03,
+	STK_TEXT_FORMAT_FONT_SIZE_LARGE = 0x04,
+	STK_TEXT_FORMAT_FONT_SIZE_SMALL = 0x08,
+	STK_TEXT_FORMAT_FONT_SIZE_RESERVED = 0x0c,
+	STK_TEXT_FORMAT_STYLE_BOLD = 0x10,
+	STK_TEXT_FORMAT_STYLE_ITALIC = 0x20,
+	STK_TEXT_FORMAT_STYLE_UNDERLINED = 0x40,
+	STK_TEXT_FORMAT_STYLE_STRIKETHROUGH = 0x80,
+};
+
 struct stk_command *stk_command_new_from_pdu(const unsigned char *pdu,
 						unsigned int len);
 void stk_command_free(struct stk_command *command);
@@ -1643,3 +1664,5 @@ const unsigned char *stk_pdu_from_response(const struct stk_response *response,
 						unsigned int *out_length);
 const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
 						unsigned int *out_length);
+char *stk_text_to_html(char *text, int text_len,
+				const unsigned char *attrs, int attrs_len);
-- 
1.6.6.1

[PATCH 2/3] test-stkutil: add unit test for html text attributes

[Kristen Carlson Accardi]

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

---
 unit/test-stkutil.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
index 8b7e254..57e894b 100644
--- a/unit/test-stkutil.c
+++ b/unit/test-stkutil.c
@@ -472,6 +472,22 @@ static inline void check_cdma_sms_tpdu(
 	check_common_byte_array(command, test);
 }
 
+static void check_text_attr_html(const struct stk_text_attribute *test,
+				char *text, const char *expected_html)
+{
+	char *html;
+
+	if (expected_html == NULL)
+		return;
+
+	html = stk_text_to_html(text, strlen(text), test->attributes,
+				test->len);
+
+	g_assert(memcmp(html, expected_html, strlen(expected_html)) == 0);
+
+	g_free(html);
+}
+
 /* Defined in TS 102.223 Section 8.72 */
 static void check_text_attr(const struct stk_text_attribute *command,
 					const struct stk_text_attribute *test)
@@ -21923,6 +21939,59 @@ static const struct envelope_test timer_expiration_data_221a = {
 	},
 };
 
+struct html_attr_test {
+	char *text;
+	struct stk_text_attribute text_attr;
+	char *html;
+};
+
+static struct html_attr_test html_attr_data_1 = {
+	.text = "EMS messages can contain italic, bold, large, small and "
+		"colored text",
+	.text_attr = {
+		.len = 20,
+		.attributes = { 0x19, 0x06, 0x20, 0x00, 0x21, 0x04, 0x10, 0x00,
+				0x27, 0x05, 0x04, 0x00, 0x2E, 0x05, 0x08, 0x00,
+				0x38, 0x07, 0x00, 0x2B },
+	},
+	.html = "EMS messages can contain <span style=\"font-style: "
+		"italic;\">italic</span>, <span style=\"font-weight: bold;\">"
+		"bold</span>, <span style=\"font-size: big;\">large</span>, "
+		"<span style=\"font-size: small;\">small</span> and "
+		"<span style=\"color: #FFFF00;background-color: #C11B17;\">"
+		"colored</span> text",
+};
+
+static struct html_attr_test html_attr_data_2 = {
+	.text = "Blue green green green",
+	.text_attr = {
+		.len = 8,
+		.attributes = {	0x00, 0x00, 0x00, 0x94, 0x00, 0x04, 0x00,
+				0x96 },
+	},
+	.html = "<span style=\"color: #0000A0;background-color: #FFFFFF;\">"
+		"Blue</span><span style=\"color: #347235;background-color: "
+		"#FFFFFF;\"> green green green</span>",
+};
+
+static struct html_attr_test html_attr_data_3 = {
+	.text = "abc",
+	.text_attr = {
+		.len = 8,
+		.attributes = { 0x00, 0x02, 0x00, 0x94, 0x01, 0x02, 0x00,
+				0x96 },
+	},
+	.html = "<span style=\"color: #347235;background-color: #FFFFFF;\">"
+		"a</span><span style=\"color: #0000A0;background-color: "
+		"#FFFFFF;\">bc</span>",
+};
+
+static void test_html_attr(gconstpointer data)
+{
+	const struct html_attr_test *test = data;
+	check_text_attr_html(&test->text_attr, test->text, test->html);
+}
+
 int main(int argc, char **argv)
 {
 	g_test_init(&argc, &argv, NULL);
@@ -23930,5 +23999,12 @@ int main(int argc, char **argv)
 	g_test_add_data_func("/teststk/Timer Expiration 2.2.1A",
 			&timer_expiration_data_221a, test_envelope_encoding);
 
+	g_test_add_data_func("/teststk/HTML Attribute Test 1",
+				&html_attr_data_1, test_html_attr);
+	g_test_add_data_func("/teststk/HTML Attribute Test 2",
+				&html_attr_data_2, test_html_attr);
+	g_test_add_data_func("/teststk/HTML Attribute Test 3",
+				&html_attr_data_3, test_html_attr);
+
 	return g_test_run();
 }
-- 
1.6.6.1

[PATCH 3/3] test-stkutil: add html attribute test for Display Text tests

[Kristen Carlson Accardi]

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

---
 unit/test-stkutil.c |  204 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 204 insertions(+), 0 deletions(-)

diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
index 57e894b..bc0e018 100644
--- a/unit/test-stkutil.c
+++ b/unit/test-stkutil.c
@@ -524,6 +524,7 @@ struct display_text_test {
 	struct stk_duration duration;
 	struct stk_text_attribute text_attr;
 	struct stk_frame_id frame_id;
+	const char *html;
 };
 
 unsigned char display_text_111[] = { 0xD0, 0x1A, 0x81, 0x03, 0x01, 0x21, 0x80,
@@ -619,6 +620,69 @@ unsigned char display_text_711[] = { 0xD0, 0x19, 0x81, 0x03, 0x01, 0x21, 0x80,
 					0x63, 0x6F, 0x6E, 0x64, 0x84, 0x02,
 					0x01, 0x0A };
 
+unsigned char display_text_811[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x00, 0xB4 };
+
+unsigned char display_text_821[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x01, 0xB4 };
+
+unsigned char display_text_831[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x02, 0xB4 };
+
+unsigned char display_text_841[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x04, 0xB4 };
+
+unsigned char display_text_851[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x08, 0xB4 };
+
+unsigned char display_text_861[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x10, 0xB4 };
+
+unsigned char display_text_871[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x20, 0xB4 };
+
+unsigned char display_text_881[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x40, 0xB4 };
+
+unsigned char display_text_891[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x80, 0xB4 };
+
 unsigned char display_text_911[] = { 0xD0, 0x10, 0x81, 0x03, 0x01, 0x21, 0x80,
 					0x82, 0x02, 0x81, 0x02, 0x8D, 0x05,
 					0x08, 0x4F, 0x60, 0x59, 0x7D };
@@ -724,6 +788,125 @@ static struct display_text_test display_text_data_711 = {
 	}
 };
 
+static struct display_text_test display_text_data_811 = {
+	.pdu = display_text_811,
+	.pdu_len = sizeof(display_text_811),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x00, 0xB4 },
+	},
+	.html = "<span style=\"color: #347235;background-color: #FFFF00;\">"
+		"Text Attribute 1</span>",
+};
+
+static struct display_text_test display_text_data_821 = {
+	.pdu = display_text_821,
+	.pdu_len = sizeof(display_text_821),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x01, 0xB4 },
+	},
+	.html = "<div style=\"text-align: center;\"><span style=\"color: "
+		"#347235;background-color: #FFFF00;\">Text Attribute 1</span>"
+		"</div>",
+};
+
+static struct display_text_test display_text_data_831 = {
+	.pdu = display_text_831,
+	.pdu_len = sizeof(display_text_831),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x02, 0xB4 },
+	},
+	.html = "<div style=\"text-align: right;\"><span style=\"color: "
+		"#347235;background-color: #FFFF00;\">Text Attribute 1</span>"
+		"</div>",
+};
+
+static struct display_text_test display_text_data_841 = {
+	.pdu = display_text_841,
+	.pdu_len = sizeof(display_text_841),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x04, 0xB4 },
+	},
+	.html = "<span style=\"font-size: big;color: #347235;background-color:"
+		" #FFFF00;\">Text Attribute 1</span>",
+};
+
+static struct display_text_test display_text_data_851 = {
+	.pdu = display_text_851,
+	.pdu_len = sizeof(display_text_851),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x08, 0xB4 },
+	},
+	.html = "<span style=\"font-size: small;color: #347235;"
+		"background-color: #FFFF00;\">Text Attribute 1</span>",
+};
+
+static struct display_text_test display_text_data_861 = {
+	.pdu = display_text_861,
+	.pdu_len = sizeof(display_text_861),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x10, 0xB4 },
+	},
+	.html = "<span style=\"font-weight: bold;color: #347235;"
+		"background-color: #FFFF00;\">Text Attribute 1</span>",
+};
+
+static struct display_text_test display_text_data_871 = {
+	.pdu = display_text_871,
+	.pdu_len = sizeof(display_text_871),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x20, 0xB4 },
+	},
+	.html = "<span style=\"font-style: italic;color: #347235;"
+		"background-color: #FFFF00;\">Text Attribute 1</span>",
+};
+
+static struct display_text_test display_text_data_881 = {
+	.pdu = display_text_881,
+	.pdu_len = sizeof(display_text_881),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x40, 0xB4 },
+	},
+	.html = "<span style=\"text-decoration: underline;color: #347235;"
+		"background-color: #FFFF00;\">Text Attribute 1</span>",
+};
+
+static struct display_text_test display_text_data_891 = {
+	.pdu = display_text_891,
+	.pdu_len = sizeof(display_text_891),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x80, 0xB4 },
+	},
+	.html = "<span style=\"text-decoration: line-through;color: #347235;"
+		"background-color: #FFFF00;\">Text Attribute 1</span>",
+};
+
 static struct display_text_test display_text_data_911 = {
 	.pdu = display_text_911,
 	.pdu_len = sizeof(display_text_911),
@@ -764,6 +947,9 @@ static void test_display_text(gconstpointer data)
 	check_duration(&command->display_text.duration, &test->duration);
 	check_text_attr(&command->display_text.text_attr,
 						&test->text_attr);
+	check_text_attr_html(&command->display_text.text_attr,
+				command->display_text.text,
+				test->html);
 	check_frame_id(&command->display_text.frame_id, &test->frame_id);
 
 	stk_command_free(command);
@@ -22018,6 +22204,24 @@ int main(int argc, char **argv)
 				&display_text_data_611, test_display_text);
 	g_test_add_data_func("/teststk/Display Text 7.1.1",
 				&display_text_data_711, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.1.1",
+				&display_text_data_811, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.2.1",
+				&display_text_data_821, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.3.1",
+				&display_text_data_831, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.4.1",
+				&display_text_data_841, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.5.1",
+				&display_text_data_851, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.6.1",
+				&display_text_data_861, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.7.1",
+				&display_text_data_871, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.8.1",
+				&display_text_data_881, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.9.1",
+				&display_text_data_891, test_display_text);
 	g_test_add_data_func("/teststk/Display Text 9.1.1",
 				&display_text_data_911, test_display_text);
 	g_test_add_data_func("/teststk/Display Text 10.1.1",
-- 
1.6.6.1

Re: [PATCH 1/3] stkutil: display text attributes as html

[Denis Kenzior]

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

Hi Kristen,

> +
> +char *stk_text_to_html(char *text, int text_len,
> +				const unsigned char *attrs, int attrs_len)
> +{
> +	GString *string = g_string_sized_new(text_len + 1);
> +	int formats[257];  /* maximum number of chars in text + 1 */
> +	int pos = 0, i, j, attr, prev_attr;
> +	guint8 start, end, code, color, len, align;
> +
> +	/* we will need formatting at the position beyond the last char */
> +	for (i = 0; i <= text_len; i++)
> +		formats[i] = STK_TEXT_FORMAT_INIT;
> +

Please note that the same formatting can be used for EMS messages
(23.040).  These messages have a fairly large max-len (255 segments *
~153 characters)  I'd like to have this function useable for EMS
messages as well.

> +	i = 0;
> +
> +	while (i < attrs_len) {

Using a for loop would be nicer here

> +		start = attrs[i++];
> +		len = attrs[i++];
> +		code = attrs[i++];

You might want to be extra paranoid here that attrs_len is a multiple of 4.

> +
> +		if (i < attrs_len)
> +			color = attrs[i++];
> +		else
> +			color = 0;
> +
> +		if (len == 0)
> +			end = text_len;
> +		else
> +			end = start + len;
> +
> +		/* sanity check values */
> +		if (start > end || end > text_len)
> +			continue;
> +
> +		/*
> +		 * if the alignment is the same as either the default
> +		 * or the last alignment used, don't set any alignment
> +		 * value.
> +		 */
> +		if (start == 0)
> +			align = STK_DEFAULT_TEXT_ALIGNMENT;

Are attributes which do not contain start = 0 valid?  If so, you might
take extra care here.

> +		else {
> +			align = (formats[start -1] & 0xFF) &
> +					STK_TEXT_FORMAT_ALIGN_MASK;
> +			if (align == STK_TEXT_FORMAT_NO_ALIGN)
> +				align = STK_DEFAULT_TEXT_ALIGNMENT;
> +		}
> +
> +		if ((code & STK_TEXT_FORMAT_ALIGN_MASK) == align)
> +			code |= STK_TEXT_FORMAT_NO_ALIGN;
> +
> +		attr = code | (color << 8);
> +
> +		for (j = start; j < end; j++)
> +			formats[j] = attr;
> +	}
> +
> +	prev_attr = STK_TEXT_FORMAT_INIT;
> +
> +	while (pos <= text_len) {

A for loop would be more readable here in my opinion

> +		attr = formats[pos];
> +		if (attr != prev_attr) {
> +			if (prev_attr != STK_TEXT_FORMAT_INIT)
> +				end_format(string, prev_attr & 0xFF);
> +
> +			if (attr != STK_TEXT_FORMAT_INIT)
> +				start_format(string, attr & 0xFF,
> +							(attr >> 8) & 0xFF);
> +
> +			prev_attr = attr;
> +		}
> +
> +		if (pos == text_len)
> +			break;
> +
> +		switch (text[pos]) {
> +		case '\n':
> +			g_string_append(string, "<br/>");
> +			break;
> +		case '\r':
> +		{
> +			g_string_append(string, "<br/>");
> +			if ((pos + 1 < text_len) && (text[pos + 1] == '\n'))
> +				pos++;
> +		}

Fallthrough on purpose?

> +		case '<':
> +			g_string_append(string, "&lt;");
> +			break;
> +		case '>':
> +			g_string_append(string, "&gt;");
> +			break;
> +		case '&':
> +			g_string_append(string, "&amp;");
> +			break;
> +		default:
> +			g_string_append_c(string, text[pos]);
> +		}
> +		pos++;
> +	}
> +
> +	/* return characters from string. Caller must free char data */
> +	return g_string_free(string, FALSE);
> +}
> diff --git a/src/stkutil.h b/src/stkutil.h
> index ca4817e..5b3a679 100644
> --- a/src/stkutil.h
> +++ b/src/stkutil.h
> @@ -1635,6 +1635,27 @@ struct stk_envelope {
>  	};
>  };
>  
> +#define STK_TEXT_FORMAT_ALIGN_MASK 0x03
> +#define STK_TEXT_FORMAT_FONT_MASK 0x0C
> +#define STK_TEXT_FORMAT_STYLE_MASK 0xF0
> +#define STK_DEFAULT_TEXT_ALIGNMENT 0x00
> +#define STK_TEXT_FORMAT_INIT -1
> +
> +/* Defined in ETSI 123 40 9.2.3.24.10.1.1 */
> +enum stk_text_format_code {
> +	STK_TEXT_FORMAT_LEFT_ALIGN = 0x00,
> +	STK_TEXT_FORMAT_CENTER_ALIGN = 0x01,
> +	STK_TEXT_FORMAT_RIGHT_ALIGN = 0x02,
> +	STK_TEXT_FORMAT_NO_ALIGN = 0x03,
> +	STK_TEXT_FORMAT_FONT_SIZE_LARGE = 0x04,
> +	STK_TEXT_FORMAT_FONT_SIZE_SMALL = 0x08,
> +	STK_TEXT_FORMAT_FONT_SIZE_RESERVED = 0x0c,
> +	STK_TEXT_FORMAT_STYLE_BOLD = 0x10,
> +	STK_TEXT_FORMAT_STYLE_ITALIC = 0x20,
> +	STK_TEXT_FORMAT_STYLE_UNDERLINED = 0x40,
> +	STK_TEXT_FORMAT_STYLE_STRIKETHROUGH = 0x80,
> +};
> +

All of these enums really belong in the .c file.  There is no other
client which can make use of them.

>  struct stk_command *stk_command_new_from_pdu(const unsigned char *pdu,
>  						unsigned int len);
>  void stk_command_free(struct stk_command *command);
> @@ -1643,3 +1664,5 @@ const unsigned char *stk_pdu_from_response(const struct stk_response *response,
>  						unsigned int *out_length);
>  const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
>  						unsigned int *out_length);
> +char *stk_text_to_html(char *text, int text_len,
> +				const unsigned char *attrs, int attrs_len);

Why not const char *text?

Regards,
-Denis

Re: [PATCH 1/3] stkutil: display text attributes as html

[Kristen Carlson Accardi]

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

On Thu, 01 Jul 2010 11:30:07 -0500
Denis Kenzior <denkenz@gmail.com> wrote:

> 
> > +		start = attrs[i++];
> > +		len = attrs[i++];
> > +		code = attrs[i++];
> 
> You might want to be extra paranoid here that attrs_len is a multiple of 4.

attrs_len does not have to be a multiple of 4.  I will add a sanity
check to attrs_len to make sure it is at least >= i + 3.

> 
> > +
> > +		if (i < attrs_len)
> > +			color = attrs[i++];
> > +		else
> > +			color = 0;
> > +
> > +		if (len == 0)
> > +			end = text_len;
> > +		else
> > +			end = start + len;
> > +
> > +		/* sanity check values */
> > +		if (start > end || end > text_len)
> > +			continue;
> > +
> > +		/*
> > +		 * if the alignment is the same as either the default
> > +		 * or the last alignment used, don't set any alignment
> > +		 * value.
> > +		 */
> > +		if (start == 0)
> > +			align = STK_DEFAULT_TEXT_ALIGNMENT;
> 
> Are attributes which do not contain start = 0 valid?  If so, you might
> take extra care here.

Yes start can be nonzero, but I don't understand what you are asking here.
If start is not equal to zero, we have already made sure it is not
greater than the end.

Thanks,
Kristen

Re: [PATCH 1/3] stkutil: display text attributes as html

[Denis Kenzior]

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

Hi Kristen,

On 07/01/2010 01:29 PM, Kristen Carlson Accardi wrote:
> On Thu, 01 Jul 2010 11:30:07 -0500
> Denis Kenzior <denkenz@gmail.com> wrote:
> 
>>
>>> +		start = attrs[i++];
>>> +		len = attrs[i++];
>>> +		code = attrs[i++];
>>
>> You might want to be extra paranoid here that attrs_len is a multiple of 4.
> 
> attrs_len does not have to be a multiple of 4.  I will add a sanity
> check to attrs_len to make sure it is at least >= i + 3.
> 

Then this code is incorrect, as it handles len 3 attrs only at the end
of the array.  Also please note that SIM Toolkit Text Attributes are
always coded on 4 bytes (see 102.223 Section 8.72 for details).  You
might want to assume 4 byte alignment or invent a data structure for
these attributes.

>>
>>> +
>>> +		if (i < attrs_len)
>>> +			color = attrs[i++];
>>> +		else
>>> +			color = 0;
>>> +
>>> +		if (len == 0)
>>> +			end = text_len;
>>> +		else
>>> +			end = start + len;
>>> +
>>> +		/* sanity check values */
>>> +		if (start > end || end > text_len)
>>> +			continue;
>>> +
>>> +		/*
>>> +		 * if the alignment is the same as either the default
>>> +		 * or the last alignment used, don't set any alignment
>>> +		 * value.
>>> +		 */
>>> +		if (start == 0)
>>> +			align = STK_DEFAULT_TEXT_ALIGNMENT;
>>
>> Are attributes which do not contain start = 0 valid?  If so, you might
>> take extra care here.
> 
> Yes start can be nonzero, but I don't understand what you are asking here.
> If start is not equal to zero, we have already made sure it is not
> greater than the end.

I'm not seeing how align gets initialized properly if start != 0.

Regards,
-Denis

Re: [PATCH 1/3] stkutil: display text attributes as html

[Kristen Carlson Accardi]

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

On Thu, 01 Jul 2010 13:54:16 -0500
Denis Kenzior <denkenz@gmail.com> wrote:

> Hi Kristen,
> 
> On 07/01/2010 01:29 PM, Kristen Carlson Accardi wrote:
> > On Thu, 01 Jul 2010 11:30:07 -0500
> > Denis Kenzior <denkenz@gmail.com> wrote:
> > 
> >>
> >>> +		start = attrs[i++];
> >>> +		len = attrs[i++];
> >>> +		code = attrs[i++];
> >>
> >> You might want to be extra paranoid here that attrs_len is a multiple of 4.
> > 
> > attrs_len does not have to be a multiple of 4.  I will add a sanity
> > check to attrs_len to make sure it is at least >= i + 3.
> > 
> 
> Then this code is incorrect, as it handles len 3 attrs only at the end
> of the array.  Also please note that SIM Toolkit Text Attributes are
> always coded on 4 bytes (see 102.223 Section 8.72 for details).  You
> might want to assume 4 byte alignment or invent a data structure for
> these attributes.

I should have said that attrs_len does not always have to be a multiple
of 4 -- it is allowed to only have 3 byte attrs as the last attribute,
so the code is correct.  However, since not all attributes are
required to be of len 4, we can't just assume attrs_len is a multiple
of 4.

> 
> >>
> >>> +
> >>> +		if (i < attrs_len)
> >>> +			color = attrs[i++];
> >>> +		else
> >>> +			color = 0;
> >>> +
> >>> +		if (len == 0)
> >>> +			end = text_len;
> >>> +		else
> >>> +			end = start + len;
> >>> +
> >>> +		/* sanity check values */
> >>> +		if (start > end || end > text_len)
> >>> +			continue;
> >>> +
> >>> +		/*
> >>> +		 * if the alignment is the same as either the default
> >>> +		 * or the last alignment used, don't set any alignment
> >>> +		 * value.
> >>> +		 */
> >>> +		if (start == 0)
> >>> +			align = STK_DEFAULT_TEXT_ALIGNMENT;
> >>
> >> Are attributes which do not contain start = 0 valid?  If so, you might
> >> take extra care here.
> > 
> > Yes start can be nonzero, but I don't understand what you are asking here.
> > If start is not equal to zero, we have already made sure it is not
> > greater than the end.
> 
> I'm not seeing how align gets initialized properly if start != 0.

I snipped that part off of my reply, sorry.  Here is the code as I
originally sent it:
 		/*
                 * if the alignment is the same as either the default
                 * or the last alignment used, don't set any alignment
                 * value.
                 */
                if (start == 0)
                        align = STK_DEFAULT_TEXT_ALIGNMENT;
                else {
                        align = (formats[start -1] & 0xFF) &
                                        STK_TEXT_FORMAT_ALIGN_MASK;
                        if (align == STK_TEXT_FORMAT_NO_ALIGN)
                                align = STK_DEFAULT_TEXT_ALIGNMENT;
                }

so if start != zero, align is set to the previous alignment value.
(formats[start - 1]).

Thanks,
Kristen

Re: [PATCH 1/3] stkutil: display text attributes as html

[Denis Kenzior]

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

On 07/01/2010 03:54 PM, Kristen Carlson Accardi wrote:
> On Thu, 01 Jul 2010 13:54:16 -0500
> Denis Kenzior <denkenz@gmail.com> wrote:
> 
>> Hi Kristen,
>>
>> On 07/01/2010 01:29 PM, Kristen Carlson Accardi wrote:
>>> On Thu, 01 Jul 2010 11:30:07 -0500
>>> Denis Kenzior <denkenz@gmail.com> wrote:
>>>
>>>>
>>>>> +		start = attrs[i++];
>>>>> +		len = attrs[i++];
>>>>> +		code = attrs[i++];
>>>>
>>>> You might want to be extra paranoid here that attrs_len is a multiple of 4.
>>>
>>> attrs_len does not have to be a multiple of 4.  I will add a sanity
>>> check to attrs_len to make sure it is at least >= i + 3.
>>>
>>
>> Then this code is incorrect, as it handles len 3 attrs only at the end
>> of the array.  Also please note that SIM Toolkit Text Attributes are
>> always coded on 4 bytes (see 102.223 Section 8.72 for details).  You
>> might want to assume 4 byte alignment or invent a data structure for
>> these attributes.
> 
> I should have said that attrs_len does not always have to be a multiple
> of 4 -- it is allowed to only have 3 byte attrs as the last attribute,
> so the code is correct.  However, since not all attributes are
> required to be of len 4, we can't just assume attrs_len is a multiple
> of 4.
> 

How do you figure this?  From 23.040 Section 9.2.3.24.10.1.1	Text Formatting

"Octet 4

This Octet may be omitted by setting the IED length accordingly."

From 102.223 Section 8.72:

"The Text Formatting is a sequence of one or several Text Formatting
items, each coded on 4 bytes.
The Text Formatting scheme used is the same as the Text Formatting
defined in TS 123 040 [27]."

So you either have to support a mixed 3 & 4 byte fields or you assume
all fields are 4 bytes.

Regards,
-Denis

Re: [PATCH 1/3] stkutil: display text attributes as html

[Andrzej Zaborowski]

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

Hi

On 1 July 2010 18:30, Denis Kenzior <denkenz@gmail.com> wrote:
>> +char *stk_text_to_html(char *text, int text_len,
>> +                             const unsigned char *attrs, int attrs_len)
>> +{
>> +     GString *string = g_string_sized_new(text_len + 1);
>> +     int formats[257];  /* maximum number of chars in text + 1 */
>> +     int pos = 0, i, j, attr, prev_attr;
>> +     guint8 start, end, code, color, len, align;
>> +
>> +     /* we will need formatting at the position beyond the last char */
>> +     for (i = 0; i <= text_len; i++)
>> +             formats[i] = STK_TEXT_FORMAT_INIT;
>> +
>
> Please note that the same formatting can be used for EMS messages
> (23.040).  These messages have a fairly large max-len (255 segments *
> ~153 characters)  I'd like to have this function useable for EMS
> messages as well.

It would have a different signature because as far as I understand a
separate array of attributes is sent in each segment (the start/len
fields can't address more than 256 characters anyway).  So that
function can be a wrapper around this function.

Regards,
Andrew

Re: [PATCH 1/3] stkutil: display text attributes as html

[Denis Kenzior]

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

Hi Andrew,

On 07/01/2010 05:10 PM, Andrzej Zaborowski wrote:
> Hi
> 
> On 1 July 2010 18:30, Denis Kenzior <denkenz@gmail.com> wrote:
>>> +char *stk_text_to_html(char *text, int text_len,
>>> +                             const unsigned char *attrs, int attrs_len)
>>> +{
>>> +     GString *string = g_string_sized_new(text_len + 1);
>>> +     int formats[257];  /* maximum number of chars in text + 1 */
>>> +     int pos = 0, i, j, attr, prev_attr;
>>> +     guint8 start, end, code, color, len, align;
>>> +
>>> +     /* we will need formatting at the position beyond the last char */
>>> +     for (i = 0; i <= text_len; i++)
>>> +             formats[i] = STK_TEXT_FORMAT_INIT;
>>> +
>>
>> Please note that the same formatting can be used for EMS messages
>> (23.040).  These messages have a fairly large max-len (255 segments *
>> ~153 characters)  I'd like to have this function useable for EMS
>> messages as well.
> 
> It would have a different signature because as far as I understand a
> separate array of attributes is sent in each segment (the start/len
> fields can't address more than 256 characters anyway).  So that
> function can be a wrapper around this function.
> 

It depends actually.  I'd like to avoid writing a separate EMS
concatenator, and re-use sms_decode_text.  This means that if possible,
I'd like this function to work on arbitrary length strings.  The text
attributes can be massaged pretty easily without knowing what's inside
the SMS message.

Regards,
-Denis

Re: [PATCH 1/3] stkutil: display text attributes as html

[Andrzej Zaborowski]

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

On 2 July 2010 00:35, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Andrew,
>
> On 07/01/2010 05:10 PM, Andrzej Zaborowski wrote:
>> Hi
>>
>> On 1 July 2010 18:30, Denis Kenzior <denkenz@gmail.com> wrote:
>>>> +char *stk_text_to_html(char *text, int text_len,
>>>> +                             const unsigned char *attrs, int attrs_len)
>>>> +{
>>>> +     GString *string = g_string_sized_new(text_len + 1);
>>>> +     int formats[257];  /* maximum number of chars in text + 1 */
>>>> +     int pos = 0, i, j, attr, prev_attr;
>>>> +     guint8 start, end, code, color, len, align;
>>>> +
>>>> +     /* we will need formatting at the position beyond the last char */
>>>> +     for (i = 0; i <= text_len; i++)
>>>> +             formats[i] = STK_TEXT_FORMAT_INIT;
>>>> +
>>>
>>> Please note that the same formatting can be used for EMS messages
>>> (23.040).  These messages have a fairly large max-len (255 segments *
>>> ~153 characters)  I'd like to have this function useable for EMS
>>> messages as well.
>>
>> It would have a different signature because as far as I understand a
>> separate array of attributes is sent in each segment (the start/len
>> fields can't address more than 256 characters anyway).  So that
>> function can be a wrapper around this function.
>>
>
> It depends actually.  I'd like to avoid writing a separate EMS
> concatenator, and re-use sms_decode_text.  This means that if possible,
> I'd like this function to work on arbitrary length strings.  The text
> attributes can be massaged pretty easily without knowing what's inside
> the SMS message.

My point is that it won't work on a message longer than 256 bytes
anyway, because you would need to either change how the attribute
start byte is coded, or supplement each attribute with information
about which segment it relates to.

Regards,
Andrew

Re: [PATCH 1/3] stkutil: display text attributes as html

[Denis Kenzior]

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

Hi Andrew,

>>> It would have a different signature because as far as I understand a
>>> separate array of attributes is sent in each segment (the start/len
>>> fields can't address more than 256 characters anyway).  So that
>>> function can be a wrapper around this function.
>>>
>>
>> It depends actually.  I'd like to avoid writing a separate EMS
>> concatenator, and re-use sms_decode_text.  This means that if possible,
>> I'd like this function to work on arbitrary length strings.  The text
>> attributes can be massaged pretty easily without knowing what's inside
>> the SMS message.
> 
> My point is that it won't work on a message longer than 256 bytes
> anyway, because you would need to either change how the attribute
> start byte is coded, or supplement each attribute with information
> about which segment it relates to.

I know, see my comment above about 'text attributes can be massaged..'

My thinking is that doing it inside this function is faster / easier in
the common case than writing a custom EMS message decoder.

Regards,
-Denis

Re: [PATCH 1/3] stkutil: display text attributes as html

[Kristen Carlson Accardi]

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

On Thu, 01 Jul 2010 16:15:35 -0500
Denis Kenzior <denkenz@gmail.com> wrote:

> On 07/01/2010 03:54 PM, Kristen Carlson Accardi wrote:
> > On Thu, 01 Jul 2010 13:54:16 -0500
> > Denis Kenzior <denkenz@gmail.com> wrote:
> > 
> >> Hi Kristen,
> >>
> >> On 07/01/2010 01:29 PM, Kristen Carlson Accardi wrote:
> >>> On Thu, 01 Jul 2010 11:30:07 -0500
> >>> Denis Kenzior <denkenz@gmail.com> wrote:
> >>>
> >>>>
> >>>>> +		start = attrs[i++];
> >>>>> +		len = attrs[i++];
> >>>>> +		code = attrs[i++];
> >>>>
> >>>> You might want to be extra paranoid here that attrs_len is a multiple of 4.
> >>>
> >>> attrs_len does not have to be a multiple of 4.  I will add a sanity
> >>> check to attrs_len to make sure it is at least >= i + 3.
> >>>
> >>
> >> Then this code is incorrect, as it handles len 3 attrs only at the end
> >> of the array.  Also please note that SIM Toolkit Text Attributes are
> >> always coded on 4 bytes (see 102.223 Section 8.72 for details).  You
> >> might want to assume 4 byte alignment or invent a data structure for
> >> these attributes.
> > 
> > I should have said that attrs_len does not always have to be a multiple
> > of 4 -- it is allowed to only have 3 byte attrs as the last attribute,
> > so the code is correct.  However, since not all attributes are
> > required to be of len 4, we can't just assume attrs_len is a multiple
> > of 4.
> > 
> 
> How do you figure this?  From 23.040 Section 9.2.3.24.10.1.1	Text Formatting
> 
> "Octet 4
> 
> This Octet may be omitted by setting the IED length accordingly."

This isn't the clearest language, so my interpretation of this is
you may omit one byte if you set the length properly.  Clearly
the spec tell you that the byte is optional, so I think it would
be wrong to assume that all are 4 bytes.  Also, since obviously
we can't support mixed 3 and 4 byte fields since we'd have no
way to know if the next byte was the start of the next attribute,
it's got to be at the end of the array.

> 
> From 102.223 Section 8.72:
> 
> "The Text Formatting is a sequence of one or several Text Formatting
> items, each coded on 4 bytes.
> The Text Formatting scheme used is the same as the Text Formatting
> defined in TS 123 040 [27]."
> 
> So you either have to support a mixed 3 & 4 byte fields or you assume
> all fields are 4 bytes.
> 
> Regards,
> -Denis

Re: [PATCH 1/3] stkutil: display text attributes as html

[Kristen Carlson Accardi]

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

On Thu, 01 Jul 2010 18:08:47 -0500
Denis Kenzior <denkenz@gmail.com> wrote:

> Hi Kristen,
> 
> >>>> Then this code is incorrect, as it handles len 3 attrs only at the end
> >>>> of the array.  Also please note that SIM Toolkit Text Attributes are
> >>>> always coded on 4 bytes (see 102.223 Section 8.72 for details).  You
> >>>> might want to assume 4 byte alignment or invent a data structure for
> >>>> these attributes.
> >>>
> >>> I should have said that attrs_len does not always have to be a multiple
> >>> of 4 -- it is allowed to only have 3 byte attrs as the last attribute,
> >>> so the code is correct.  However, since not all attributes are
> >>> required to be of len 4, we can't just assume attrs_len is a multiple
> >>> of 4.
> >>>
> >>
> >> How do you figure this?  From 23.040 Section 9.2.3.24.10.1.1	Text Formatting
> >>
> >> "Octet 4
> >>
> >> This Octet may be omitted by setting the IED length accordingly."
> > 
> > This isn't the clearest language, so my interpretation of this is
> > you may omit one byte if you set the length properly.  Clearly
> > the spec tell you that the byte is optional, so I think it would
> > be wrong to assume that all are 4 bytes.  Also, since obviously
> > we can't support mixed 3 and 4 byte fields since we'd have no
> > way to know if the next byte was the start of the next attribute,
> > it's got to be at the end of the array.
> 
> Actually it is not.  Each Text attribute is put into an SMS header, with
> the header having an IEI (Information Element Identifier), Length and
> Data.  See SMS_IEI_TEXT_FORMAT in smsutil.h
> 
> Hence, when an EMS PDU is parsed, you know whether this is a 3 byte or a
> 4 byte element.
> 
> Regards,
> -Denis

The way this function is - we have no knowledge of the pdu.
Are you wanting to change the arguments to this function to include
an sms header?  or a pdu? Or are you saying the caller is responsible 
for padding out the text attribute into a 4 byte attribute?

Re: [PATCH 1/3] stkutil: display text attributes as html

[Denis Kenzior]

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

Hi Kristen,

>>>> Then this code is incorrect, as it handles len 3 attrs only at the end
>>>> of the array.  Also please note that SIM Toolkit Text Attributes are
>>>> always coded on 4 bytes (see 102.223 Section 8.72 for details).  You
>>>> might want to assume 4 byte alignment or invent a data structure for
>>>> these attributes.
>>>
>>> I should have said that attrs_len does not always have to be a multiple
>>> of 4 -- it is allowed to only have 3 byte attrs as the last attribute,
>>> so the code is correct.  However, since not all attributes are
>>> required to be of len 4, we can't just assume attrs_len is a multiple
>>> of 4.
>>>
>>
>> How do you figure this?  From 23.040 Section 9.2.3.24.10.1.1	Text Formatting
>>
>> "Octet 4
>>
>> This Octet may be omitted by setting the IED length accordingly."
> 
> This isn't the clearest language, so my interpretation of this is
> you may omit one byte if you set the length properly.  Clearly
> the spec tell you that the byte is optional, so I think it would
> be wrong to assume that all are 4 bytes.  Also, since obviously
> we can't support mixed 3 and 4 byte fields since we'd have no
> way to know if the next byte was the start of the next attribute,
> it's got to be at the end of the array.

Actually it is not.  Each Text attribute is put into an SMS header, with
the header having an IEI (Information Element Identifier), Length and
Data.  See SMS_IEI_TEXT_FORMAT in smsutil.h

Hence, when an EMS PDU is parsed, you know whether this is a 3 byte or a
4 byte element.

Regards,
-Denis

Re: [PATCH 1/3] stkutil: display text attributes as html

[Denis Kenzior]

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

Hi Kristen,

On 07/01/2010 06:05 PM, Kristen Carlson Accardi wrote:
> On Thu, 01 Jul 2010 18:08:47 -0500
> Denis Kenzior <denkenz@gmail.com> wrote:
> 
>> Hi Kristen,
>>
>>>>>> Then this code is incorrect, as it handles len 3 attrs only at the end
>>>>>> of the array.  Also please note that SIM Toolkit Text Attributes are
>>>>>> always coded on 4 bytes (see 102.223 Section 8.72 for details).  You
>>>>>> might want to assume 4 byte alignment or invent a data structure for
>>>>>> these attributes.
>>>>>
>>>>> I should have said that attrs_len does not always have to be a multiple
>>>>> of 4 -- it is allowed to only have 3 byte attrs as the last attribute,
>>>>> so the code is correct.  However, since not all attributes are
>>>>> required to be of len 4, we can't just assume attrs_len is a multiple
>>>>> of 4.
>>>>>
>>>>
>>>> How do you figure this?  From 23.040 Section 9.2.3.24.10.1.1	Text Formatting
>>>>
>>>> "Octet 4
>>>>
>>>> This Octet may be omitted by setting the IED length accordingly."
>>>
>>> This isn't the clearest language, so my interpretation of this is
>>> you may omit one byte if you set the length properly.  Clearly
>>> the spec tell you that the byte is optional, so I think it would
>>> be wrong to assume that all are 4 bytes.  Also, since obviously
>>> we can't support mixed 3 and 4 byte fields since we'd have no
>>> way to know if the next byte was the start of the next attribute,
>>> it's got to be at the end of the array.
>>
>> Actually it is not.  Each Text attribute is put into an SMS header, with
>> the header having an IEI (Information Element Identifier), Length and
>> Data.  See SMS_IEI_TEXT_FORMAT in smsutil.h
>>
>> Hence, when an EMS PDU is parsed, you know whether this is a 3 byte or a
>> 4 byte element.
>>
>> Regards,
>> -Denis
> 
> The way this function is - we have no knowledge of the pdu.
> Are you wanting to change the arguments to this function to include
> an sms header?  or a pdu? Or are you saying the caller is responsible 
> for padding out the text attribute into a 4 byte attribute?

Correct, assume the caller will always give you quadruples.  3 byte
values can only happen in EMS, and those are wrapped in user headers.
It is not a single contiguous array.  Some external entity will need to
appropriately format the text attribute array for your consumption in
this case.

Regards,
-Denis