[PATCH v3 0/3] Support character literals

[PATCH v3 0/3] Support character literals

[Anton Staaf]

    These patches add simple and escaped character literal parsing support
to the dtc for cell lists and bytestrings.  The first patch refactors the
string parsing code in data.c to expose the more primitive character parsing
code for use in the later patches.  I have left the bytestring support in
place but my hope is that it is now separate enough to be discussed
independantly of the refactor and cell list changes.

    Thanks,
        Anton

Changes in v3:
- Make octal and hex parsing code static to util.c
- Add test cases for both cell lists and bytestings w/ character literals
- Better error handling and simpler lexing
- Removed changes to out of date manual.txt
- Simplify use of get_escape_char in data_copy_escape_string
- Remove get_escape_char_exact

Changes in v2:
- Move the refactor of data.c to a separate patch
- Add support for character literals in cell lists
- Merge normal and escaped literal support for bytestrings into single patch

Anton Staaf (3):
  dtc: Refactor character literal parsing code
  dtc: Support character literals in cell lists
  dtc: Support character literals in bytestrings

 Documentation/dts-format.txt |    7 ++-
 data.c                       |   85 ++----------------------------------
 dtc-lexer.l                  |   15 ++++++
 dtc-parser.y                 |   30 +++++++++++++
 tests/.gitignore             |    1 +
 tests/Makefile.tests         |    1 +
 tests/char_literal.c         |   58 ++++++++++++++++++++++++
 tests/char_literal.dts       |    6 +++
 tests/run_tests.sh           |    3 +
 tests/testdata.h             |    6 +++
 util.c                       |   99 ++++++++++++++++++++++++++++++++++++++++++
 util.h                       |    8 +++
 12 files changed, 235 insertions(+), 84 deletions(-)
 create mode 100644 tests/char_literal.c
 create mode 100644 tests/char_literal.dts

-- 
1.7.3.1

[PATCH v3 1/3] dtc: Refactor character literal parsing code

[Anton Staaf]

Move the parsing of hex, octal and escaped characters from data.c
to util.c where it can be used for character literal parsing within
strings as well as for stand alone C style character literals.

Signed-off-by: Anton Staaf <robotboy-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>
Cc: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 data.c |   85 ++----------------------------------------------------
 util.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 util.h |    8 +++++
 3 files changed, 111 insertions(+), 81 deletions(-)

diff --git a/data.c b/data.c
index fe555e8..b5f3066 100644
--- a/data.c
+++ b/data.c
@@ -68,40 +68,6 @@ struct data data_copy_mem(const char *mem, int len)
 	return d;
 }
 
-static char get_oct_char(const char *s, int *i)
-{
-	char x[4];
-	char *endx;
-	long val;
-
-	x[3] = '\0';
-	strncpy(x, s + *i, 3);
-
-	val = strtol(x, &endx, 8);
-
-	assert(endx > x);
-
-	(*i) += endx - x;
-	return val;
-}
-
-static char get_hex_char(const char *s, int *i)
-{
-	char x[3];
-	char *endx;
-	long val;
-
-	x[2] = '\0';
-	strncpy(x, s + *i, 2);
-
-	val = strtol(x, &endx, 16);
-	if (!(endx  > x))
-		die("\\x used with no following hex digits\n");
-
-	(*i) += endx - x;
-	return val;
-}
-
 struct data data_copy_escape_string(const char *s, int len)
 {
 	int i = 0;
@@ -114,53 +80,10 @@ struct data data_copy_escape_string(const char *s, int len)
 	while (i < len) {
 		char c = s[i++];
 
-		if (c != '\\') {
-			q[d.len++] = c;
-			continue;
-		}
-
-		c = s[i++];
-		assert(c);
-		switch (c) {
-		case 'a':
-			q[d.len++] = '\a';
-			break;
-		case 'b':
-			q[d.len++] = '\b';
-			break;
-		case 't':
-			q[d.len++] = '\t';
-			break;
-		case 'n':
-			q[d.len++] = '\n';
-			break;
-		case 'v':
-			q[d.len++] = '\v';
-			break;
-		case 'f':
-			q[d.len++] = '\f';
-			break;
-		case 'r':
-			q[d.len++] = '\r';
-			break;
-		case '0':
-		case '1':
-		case '2':
-		case '3':
-		case '4':
-		case '5':
-		case '6':
-		case '7':
-			i--; /* need to re-read the first digit as
-			      * part of the octal value */
-			q[d.len++] = get_oct_char(s, &i);
-			break;
-		case 'x':
-			q[d.len++] = get_hex_char(s, &i);
-			break;
-		default:
-			q[d.len++] = c;
-		}
+		if (c == '\\')
+			c = get_escape_char(s, &i);
+
+		q[d.len++] = c;
 	}
 
 	q[d.len++] = '\0';
diff --git a/util.c b/util.c
index 994436f..6d07292 100644
--- a/util.c
+++ b/util.c
@@ -25,6 +25,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <string.h>
+#include <assert.h>
 
 #include "util.h"
 
@@ -85,3 +86,101 @@ int util_is_printable_string(const void *data, int len)
 
 	return 1;
 }
+
+/*
+ * Parse a octal encoded character starting at index i in string s.  The
+ * resulting character will be returned and the index i will be updated to
+ * point at the character directly after the end of the encoding, this may be
+ * the '\0' terminator of the string.
+ */
+static char get_oct_char(const char *s, int *i)
+{
+	char x[4];
+	char *endx;
+	long val;
+
+	x[3] = '\0';
+	strncpy(x, s + *i, 3);
+
+	val = strtol(x, &endx, 8);
+
+	assert(endx > x);
+
+	(*i) += endx - x;
+	return val;
+}
+
+/*
+ * Parse a hexadecimal encoded character starting at index i in string s.  The
+ * resulting character will be returned and the index i will be updated to
+ * point at the character directly after the end of the encoding, this may be
+ * the '\0' terminator of the string.
+ */
+static char get_hex_char(const char *s, int *i)
+{
+	char x[3];
+	char *endx;
+	long val;
+
+	x[2] = '\0';
+	strncpy(x, s + *i, 2);
+
+	val = strtol(x, &endx, 16);
+	if (!(endx  > x))
+		die("\\x used with no following hex digits\n");
+
+	(*i) += endx - x;
+	return val;
+}
+
+char get_escape_char(const char *s, int *i)
+{
+	char	c = s[*i];
+	int	j = *i + 1;
+	char	val;
+
+	assert(c);
+	switch (c) {
+	case 'a':
+		val = '\a';
+		break;
+	case 'b':
+		val = '\b';
+		break;
+	case 't':
+		val = '\t';
+		break;
+	case 'n':
+		val = '\n';
+		break;
+	case 'v':
+		val = '\v';
+		break;
+	case 'f':
+		val = '\f';
+		break;
+	case 'r':
+		val = '\r';
+		break;
+	case '0':
+	case '1':
+	case '2':
+	case '3':
+	case '4':
+	case '5':
+	case '6':
+	case '7':
+		j--; /* need to re-read the first digit as
+		      * part of the octal value */
+		val = get_oct_char(s, &j);
+		break;
+	case 'x':
+		val = get_hex_char(s, &j);
+		break;
+	default:
+		val = c;
+	}
+
+	(*i) = j;
+	return val;
+}
diff --git a/util.h b/util.h
index cc68933..f251480 100644
--- a/util.h
+++ b/util.h
@@ -64,4 +64,12 @@ extern char *join_path(const char *path, const char *name);
  * @return 1 if a valid printable string, 0 if not */
 int util_is_printable_string(const void *data, int len);
 
+/*
+ * Parse an escaped character starting at index i in string s.  The resulting
+ * character will be returned and the index i will be updated to point at the
+ * character directly after the end of the encoding, this may be the '\0'
+ * terminator of the string.
+ */
+char get_escape_char(const char *s, int *i);
+
 #endif /* _UTIL_H */
-- 
1.7.3.1

[PATCH v3 2/3] dtc: Support character literals in cell lists

[Anton Staaf]

With this patch the following property assignment:

    property = <0x12345678 'a' '\r' 100>;

is equivalent to:

    property = <0x12345678 0x00000061 0x0000000D 0x00000064>

Signed-off-by: Anton Staaf <robotboy-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>
Cc: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 Documentation/dts-format.txt |    2 +-
 dtc-lexer.l                  |    8 ++++++
 dtc-parser.y                 |   26 +++++++++++++++++++++
 tests/.gitignore             |    1 +
 tests/Makefile.tests         |    1 +
 tests/char_literal.c         |   50 ++++++++++++++++++++++++++++++++++++++++++
 tests/char_literal.dts       |    5 ++++
 tests/run_tests.sh           |    3 ++
 tests/testdata.h             |    6 +++++
 9 files changed, 101 insertions(+), 1 deletions(-)
 create mode 100644 tests/char_literal.c
 create mode 100644 tests/char_literal.dts

diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
index a655b87..eae8b76 100644
--- a/Documentation/dts-format.txt
+++ b/Documentation/dts-format.txt
@@ -33,7 +33,7 @@ Property values may be defined as an array of 32-bit integer cells, as
 NUL-terminated strings, as bytestrings or a combination of these.
 
 * Arrays of cells are represented by angle brackets surrounding a
-  space separated list of C-style integers
+  space separated list of C-style integers or character literals.
 
 	e.g. interrupts = <17 0xc>;
 
diff --git a/dtc-lexer.l b/dtc-lexer.l
index e866ea5..94151da 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -29,6 +29,7 @@ PROPNODECHAR	[a-zA-Z0-9,._+*#?@-]
 PATHCHAR	({PROPNODECHAR}|[/])
 LABEL		[a-zA-Z_][a-zA-Z0-9_]*
 STRING		\"([^\\"]|\\.)*\"
+CHAR_LITERAL	'[^']+'|'\\''
 WS		[[:space:]]
 COMMENT		"/*"([^*]|\*+[^*/])*\*+"/"
 LINECOMMENT	"//".*\n
@@ -109,6 +110,13 @@ static int pop_input_file(void);
 			return DT_LITERAL;
 		}
 
+<V1>{CHAR_LITERAL}	{
+			yytext[yyleng-1] = '\0';
+			yylval.literal = xstrdup(yytext+1);
+			DPRINT("Character literal: %s\n", yylval.literal);
+			return DT_CHAR_LITERAL;
+		}
+
 <*>\&{LABEL}	{	/* label reference */
 			DPRINT("Ref: %s\n", yytext+1);
 			yylval.labelref = xstrdup(yytext+1);
diff --git a/dtc-parser.y b/dtc-parser.y
index 5e84a67..49bf208 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -34,6 +34,7 @@ extern struct boot_info *the_boot_info;
 extern int treesource_error;
 
 static unsigned long long eval_literal(const char *s, int base, int bits);
+static unsigned char eval_char_literal(const char *s);
 %}
 
 %union {
@@ -57,6 +58,7 @@ static unsigned long long eval_literal(const char *s, int base, int bits);
 %token DT_MEMRESERVE
 %token <propnodename> DT_PROPNODENAME
 %token <literal> DT_LITERAL
+%token <literal> DT_CHAR_LITERAL
 %token <cbase> DT_BASE
 %token <byte> DT_BYTE
 %token <data> DT_STRING
@@ -265,6 +267,10 @@ cellval:
 		{
 			$$ = eval_literal($1, 0, 32);
 		}
+	| DT_CHAR_LITERAL
+		{
+			$$ = eval_char_literal($1);
+		}
 	;
 
 bytestring:
@@ -343,3 +349,23 @@ static unsigned long long eval_literal(const char *s, int base, int bits)
 		print_error("bad literal");
 	return val;
 }
+
+static unsigned char eval_char_literal(const char *s)
+{
+	int i = 1;
+	char c = s[0];
+
+	/*
+	 * If the first character in the character literal is a \ then process
+	 * the remaining characters as an escape encoding. If the first
+	 * character is neither an escape or a terminator it should be the only
+	 * character in the literal and will be returned.
+	 */
+	if (c == '\\')
+		c = get_escape_char(s, &i);
+
+	if (s[i] != '\0')
+		print_error("malformed character literal");
+
+	return c;
+}
diff --git a/tests/.gitignore b/tests/.gitignore
index f4e58b2..a3e9bd1 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -4,6 +4,7 @@
 /add_subnode_with_nops
 /asm_tree_dump
 /boot-cpuid
+/char_literal
 /del_node
 /del_property
 /dtbs_equal_ordered
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index c564e72..e718b63 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -5,6 +5,7 @@ LIB_TESTS_L = get_mem_rsv \
 	node_offset_by_prop_value node_offset_by_phandle \
 	node_check_compatible node_offset_by_compatible \
 	get_alias \
+	char_literal \
 	notfound \
 	setprop_inplace nop_property nop_node \
 	sw_tree1 \
diff --git a/tests/char_literal.c b/tests/char_literal.c
new file mode 100644
index 0000000..150f2a0
--- /dev/null
+++ b/tests/char_literal.c
@@ -0,0 +1,50 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Testcase for character literals in dtc
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ * Copyright (C) 2011 The Chromium Authors. All rights reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "tests.h"
+#include "testdata.h"
+
+int main(int argc, char *argv[])
+{
+	void *fdt;
+	uint32_t expected_cells[5];
+
+	expected_cells[0] = cpu_to_fdt32((unsigned char)TEST_CHAR1);
+	expected_cells[1] = cpu_to_fdt32((unsigned char)TEST_CHAR2);
+	expected_cells[2] = cpu_to_fdt32((unsigned char)TEST_CHAR3);
+	expected_cells[3] = cpu_to_fdt32((unsigned char)TEST_CHAR4);
+	expected_cells[4] = cpu_to_fdt32((unsigned char)TEST_CHAR5);
+
+	test_init(argc, argv);
+	fdt = load_blob_arg(argc, argv);
+
+	check_getprop(fdt, 0, "char-literal-cells",
+		      sizeof(expected_cells), expected_cells);
+
+	PASS();
+}
diff --git a/tests/char_literal.dts b/tests/char_literal.dts
new file mode 100644
index 0000000..22e17ed
--- /dev/null
+++ b/tests/char_literal.dts
@@ -0,0 +1,5 @@
+/dts-v1/;
+
+/ {
+	char-literal-cells = <'\r' 'b' '\0' '\'' '\xff'>;
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 72dda32..1246df1 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -206,6 +206,9 @@ dtc_tests () {
     run_dtc_test -I dts -O dtb -o dtc_escapes.test.dtb escapes.dts
     run_test string_escapes dtc_escapes.test.dtb
 
+    run_dtc_test -I dts -O dtb -o dtc_char_literal.test.dtb char_literal.dts
+    run_test char_literal dtc_char_literal.test.dtb
+
     run_dtc_test -I dts -O dtb -o dtc_extra-terminating-null.test.dtb extra-terminating-null.dts
     run_test extra-terminating-null dtc_extra-terminating-null.test.dtb
 
diff --git a/tests/testdata.h b/tests/testdata.h
index 5b5a9a3..d4c6759 100644
--- a/tests/testdata.h
+++ b/tests/testdata.h
@@ -19,6 +19,12 @@
 #define TEST_STRING_2	"nastystring: \a\b\t\n\v\f\r\\\""
 #define TEST_STRING_3	"\xde\xad\xbe\xef"
 
+#define TEST_CHAR1	'\r'
+#define TEST_CHAR2	'b'
+#define TEST_CHAR3	'\0'
+#define TEST_CHAR4	'\''
+#define TEST_CHAR5	'\xff'
+
 #ifndef __ASSEMBLY__
 extern struct fdt_header _test_tree1;
 extern struct fdt_header _truncated_property;
-- 
1.7.3.1

[PATCH v3 3/3] dtc: Support character literals in bytestrings

[Anton Staaf]

With this patch the following property assignment:

    property = ['a' 2b '\r'];

is equivalent to:

    property = [61 2b 0d];

Signed-off-by: Anton Staaf <robotboy-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>
Cc: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---
 Documentation/dts-format.txt |    5 +++--
 dtc-lexer.l                  |    7 +++++++
 dtc-parser.y                 |    4 ++++
 tests/char_literal.c         |    8 ++++++++
 tests/char_literal.dts       |    1 +
 5 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
index eae8b76..555bd89 100644
--- a/Documentation/dts-format.txt
+++ b/Documentation/dts-format.txt
@@ -48,11 +48,12 @@ NUL-terminated strings, as bytestrings or a combination of these.
 	e.g. compatible = "simple-bus";
 
 * A bytestring is enclosed in square brackets [] with each byte
-  represented by two hexadecimal digits.  Spaces between each byte are
-  optional.
+  represented by two hexadecimal digits or a character literal.
+  Spaces between each byte or character literal are optional.
 
 	e.g. local-mac-address = [00 00 12 34 56 78]; or equivalently
 	     local-mac-address = [000012345678];
+	e.g. keymap = ['a' 'b' 'c' 'd'];
 
 * Values may have several comma-separated components, which are
   concatenated together.
diff --git a/dtc-lexer.l b/dtc-lexer.l
index 94151da..11082c2 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -136,6 +136,13 @@ static int pop_input_file(void);
 			return DT_BYTE;
 		}
 
+<BYTESTRING>{CHAR_LITERAL} {
+			yytext[yyleng-1] = '\0';
+			yylval.literal = xstrdup(yytext+1);
+			DPRINT("Character literal: %s\n", yylval.literal);
+			return DT_CHAR_LITERAL;
+		}
+
 <BYTESTRING>"]"	{
 			DPRINT("/BYTESTRING\n");
 			BEGIN_DEFAULT();
diff --git a/dtc-parser.y b/dtc-parser.y
index 49bf208..020222f 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -286,6 +286,10 @@ bytestring:
 		{
 			$$ = data_add_marker($1, LABEL, $2);
 		}
+	| bytestring DT_CHAR_LITERAL
+		{
+			$$ = data_append_byte($1, eval_char_literal($2));
+		}
 	;
 
 subnodes:
diff --git a/tests/char_literal.c b/tests/char_literal.c
index 150f2a0..c6e2405 100644
--- a/tests/char_literal.c
+++ b/tests/char_literal.c
@@ -33,6 +33,11 @@ int main(int argc, char *argv[])
 {
 	void *fdt;
 	uint32_t expected_cells[5];
+	uint8_t expected_bytes[5] = {TEST_CHAR1,
+				     TEST_CHAR2,
+				     TEST_CHAR3,
+				     TEST_CHAR4,
+				     TEST_CHAR5};
 
 	expected_cells[0] = cpu_to_fdt32((unsigned char)TEST_CHAR1);
 	expected_cells[1] = cpu_to_fdt32((unsigned char)TEST_CHAR2);
@@ -46,5 +51,8 @@ int main(int argc, char *argv[])
 	check_getprop(fdt, 0, "char-literal-cells",
 		      sizeof(expected_cells), expected_cells);
 
+	check_getprop(fdt, 0, "char-literal-bytes",
+		      sizeof(expected_bytes), expected_bytes);
+
 	PASS();
 }
diff --git a/tests/char_literal.dts b/tests/char_literal.dts
index 22e17ed..9baba5c 100644
--- a/tests/char_literal.dts
+++ b/tests/char_literal.dts
@@ -2,4 +2,5 @@
 
 / {
 	char-literal-cells = <'\r' 'b' '\0' '\'' '\xff'>;
+	char-literal-bytes = ['\r' 'b' '\0' '\'' '\xff'];
 };
-- 
1.7.3.1

Re: [PATCH v3 1/3] dtc: Refactor character literal parsing code

[David Gibson]

On Thu, Sep 08, 2011 at 02:39:15PM -0700, Anton Staaf wrote:
> Move the parsing of hex, octal and escaped characters from data.c
> to util.c where it can be used for character literal parsing within
> strings as well as for stand alone C style character literals.
> 
> Signed-off-by: Anton Staaf <robotboy-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3 2/3] dtc: Support character literals in cell lists

[David Gibson]

On Thu, Sep 08, 2011 at 02:39:16PM -0700, Anton Staaf wrote:
> With this patch the following property assignment:
> 
>     property = <0x12345678 'a' '\r' 100>;
> 
> is equivalent to:
> 
>     property = <0x12345678 0x00000061 0x0000000D 0x00000064>

One tiny nit..

[snip]
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index e866ea5..94151da 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -29,6 +29,7 @@ PROPNODECHAR	[a-zA-Z0-9,._+*#?@-]
>  PATHCHAR	({PROPNODECHAR}|[/])
>  LABEL		[a-zA-Z_][a-zA-Z0-9_]*
>  STRING		\"([^\\"]|\\.)*\"
> +CHAR_LITERAL	'[^']+'|'\\''

I think this should be
	'([^']|\\')*'

On the same grounds I described before.  That does mean
eval_char_literal() will have to cope with the case of ''.

[snip]
> diff --git a/tests/char_literal.c b/tests/char_literal.c
> new file mode 100644
> index 0000000..150f2a0
> --- /dev/null
> +++ b/tests/char_literal.c
> @@ -0,0 +1,50 @@
> +/*
> + * libfdt - Flat Device Tree manipulation
> + *	Testcase for character literals in dtc
> + * Copyright (C) 2006 David Gibson, IBM Corporation.
> + * Copyright (C) 2011 The Chromium Authors. All rights reserved.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 2.1 of
> + * the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdint.h>
> +
> +#include <fdt.h>
> +#include <libfdt.h>
> +
> +#include "tests.h"
> +#include "testdata.h"
> +
> +int main(int argc, char *argv[])
> +{
> +	void *fdt;
> +	uint32_t expected_cells[5];
> +
> +	expected_cells[0] = cpu_to_fdt32((unsigned char)TEST_CHAR1);
> +	expected_cells[1] = cpu_to_fdt32((unsigned char)TEST_CHAR2);
> +	expected_cells[2] = cpu_to_fdt32((unsigned char)TEST_CHAR3);
> +	expected_cells[3] = cpu_to_fdt32((unsigned char)TEST_CHAR4);
> +	expected_cells[4] = cpu_to_fdt32((unsigned char)TEST_CHAR5);
> +
> +	test_init(argc, argv);
> +	fdt = load_blob_arg(argc, argv);
> +
> +	check_getprop(fdt, 0, "char-literal-cells",
> +		      sizeof(expected_cells), expected_cells);
> +
> +	PASS();
> +}

Looks good, this is more or less exactly what I'd enivisaged for a
testcase.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3 3/3] dtc: Support character literals in bytestrings

[David Gibson]

On Thu, Sep 08, 2011 at 02:39:17PM -0700, Anton Staaf wrote:
> With this patch the following property assignment:
> 
>     property = ['a' 2b '\r'];
> 
> is equivalent to:
> 
>     property = [61 2b 0d];

[snip]
> +<BYTESTRING>{CHAR_LITERAL} {

You should just be able to make the existing rule a <*> one,
rather than having a separate rule for BYTESTRING context.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3 3/3] dtc: Support character literals in bytestrings

[Anton Staaf]

On Thu, Sep 8, 2011 at 6:15 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Thu, Sep 08, 2011 at 02:39:17PM -0700, Anton Staaf wrote:
>> With this patch the following property assignment:
>>
>>     property = ['a' 2b '\r'];
>>
>> is equivalent to:
>>
>>     property = [61 2b 0d];
>
> [snip]
>> +<BYTESTRING>{CHAR_LITERAL} {
>
> You should just be able to make the existing rule a <*> one,
> rather than having a separate rule for BYTESTRING context.

I thought about that too, but I was concerned that it would then allow
character literals in properties outside of the cell list or
bytestring syntax.  Which I suppose is exactly what you had suggested
before now that I think about it.  And would end up being no more
ambiguous, and possibly more internally consistent.  I'll make the
change and add a test case that shows that functionality as well if
that makes sense to you.

Thanks,
    Anton

> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>

Re: [PATCH v3 2/3] dtc: Support character literals in cell lists

[Anton Staaf]

On Thu, Sep 8, 2011 at 6:12 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Thu, Sep 08, 2011 at 02:39:16PM -0700, Anton Staaf wrote:
>> With this patch the following property assignment:
>>
>>     property = <0x12345678 'a' '\r' 100>;
>>
>> is equivalent to:
>>
>>     property = <0x12345678 0x00000061 0x0000000D 0x00000064>
>
> One tiny nit..
>
> [snip]
>> diff --git a/dtc-lexer.l b/dtc-lexer.l
>> index e866ea5..94151da 100644
>> --- a/dtc-lexer.l
>> +++ b/dtc-lexer.l
>> @@ -29,6 +29,7 @@ PROPNODECHAR        [a-zA-Z0-9,._+*#?@-]
>>  PATHCHAR     ({PROPNODECHAR}|[/])
>>  LABEL                [a-zA-Z_][a-zA-Z0-9_]*
>>  STRING               \"([^\\"]|\\.)*\"
>> +CHAR_LITERAL '[^']+'|'\\''
>
> I think this should be
>        '([^']|\\')*'
>
> On the same grounds I described before.  That does mean
> eval_char_literal() will have to cope with the case of ''.

OK, I could also make it '([^']|\\')+'

That way the empty character literal wouldn't be allowed and it still
allows all of the cases we care about.

> [snip]
>> diff --git a/tests/char_literal.c b/tests/char_literal.c
>> new file mode 100644
>> index 0000000..150f2a0
>> --- /dev/null
>> +++ b/tests/char_literal.c
>> @@ -0,0 +1,50 @@
>> +/*
>> + * libfdt - Flat Device Tree manipulation
>> + *   Testcase for character literals in dtc
>> + * Copyright (C) 2006 David Gibson, IBM Corporation.
>> + * Copyright (C) 2011 The Chromium Authors. All rights reserved.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public License
>> + * as published by the Free Software Foundation; either version 2.1 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <stdint.h>
>> +
>> +#include <fdt.h>
>> +#include <libfdt.h>
>> +
>> +#include "tests.h"
>> +#include "testdata.h"
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +     void *fdt;
>> +     uint32_t expected_cells[5];
>> +
>> +     expected_cells[0] = cpu_to_fdt32((unsigned char)TEST_CHAR1);
>> +     expected_cells[1] = cpu_to_fdt32((unsigned char)TEST_CHAR2);
>> +     expected_cells[2] = cpu_to_fdt32((unsigned char)TEST_CHAR3);
>> +     expected_cells[3] = cpu_to_fdt32((unsigned char)TEST_CHAR4);
>> +     expected_cells[4] = cpu_to_fdt32((unsigned char)TEST_CHAR5);
>> +
>> +     test_init(argc, argv);
>> +     fdt = load_blob_arg(argc, argv);
>> +
>> +     check_getprop(fdt, 0, "char-literal-cells",
>> +                   sizeof(expected_cells), expected_cells);
>> +
>> +     PASS();
>> +}
>
> Looks good, this is more or less exactly what I'd enivisaged for a
> testcase.

Glad to hear it.

Thanks,
    Anton

> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>

Re: [PATCH v3 3/3] dtc: Support character literals in bytestrings

[David Gibson]

On Thu, Sep 08, 2011 at 11:37:21PM -0700, Anton Staaf wrote:
> On Thu, Sep 8, 2011 at 6:15 PM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Thu, Sep 08, 2011 at 02:39:17PM -0700, Anton Staaf wrote:
> >> With this patch the following property assignment:
> >>
> >>     property = ['a' 2b '\r'];
> >>
> >> is equivalent to:
> >>
> >>     property = [61 2b 0d];
> >
> > [snip]
> >> +<BYTESTRING>{CHAR_LITERAL} {
> >
> > You should just be able to make the existing rule a <*> one,
> > rather than having a separate rule for BYTESTRING context.
> 
> I thought about that too, but I was concerned that it would then allow
> character literals in properties outside of the cell list or
> bytestring syntax.  Which I suppose is exactly what you had suggested
> before now that I think about it.  And would end up being no more
> ambiguous, and possibly more internally consistent.  I'll make the
> change and add a test case that shows that functionality as well if
> that makes sense to you.

Well, it would allow character literals to be *lexed* in other places,
but the parser would still reject them outside of the (so far)
intended contexts without further changes.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3 2/3] dtc: Support character literals in cell lists

[David Gibson]

On Thu, Sep 08, 2011 at 11:40:59PM -0700, Anton Staaf wrote:
> On Thu, Sep 8, 2011 at 6:12 PM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Thu, Sep 08, 2011 at 02:39:16PM -0700, Anton Staaf wrote:
> >> With this patch the following property assignment:
> >>
> >>     property = <0x12345678 'a' '\r' 100>;
> >>
> >> is equivalent to:
> >>
> >>     property = <0x12345678 0x00000061 0x0000000D 0x00000064>
> >
> > One tiny nit..
> >
> > [snip]
> >> diff --git a/dtc-lexer.l b/dtc-lexer.l
> >> index e866ea5..94151da 100644
> >> --- a/dtc-lexer.l
> >> +++ b/dtc-lexer.l
> >> @@ -29,6 +29,7 @@ PROPNODECHAR        [a-zA-Z0-9,._+*#?@-]
> >>  PATHCHAR     ({PROPNODECHAR}|[/])
> >>  LABEL                [a-zA-Z_][a-zA-Z0-9_]*
> >>  STRING               \"([^\\"]|\\.)*\"
> >> +CHAR_LITERAL '[^']+'|'\\''
> >
> > I think this should be
> >        '([^']|\\')*'
> >
> > On the same grounds I described before.  That does mean
> > eval_char_literal() will have to cope with the case of ''.
> 
> OK, I could also make it '([^']|\\')+'
> 
> That way the empty character literal wouldn't be allowed and it still
> allows all of the cases we care about.

You could, but the same reasoning applies here:  An error message
saying "empty character literal" is almost certainly more useful than
"syntax error, Unexpected '" or whatever bison will give us on its
own.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3 2/3] dtc: Support character literals in cell lists

[Anton Staaf]

On Fri, Sep 9, 2011 at 12:03 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Thu, Sep 08, 2011 at 11:40:59PM -0700, Anton Staaf wrote:
>> On Thu, Sep 8, 2011 at 6:12 PM, David Gibson
>> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>> > On Thu, Sep 08, 2011 at 02:39:16PM -0700, Anton Staaf wrote:
>> >> With this patch the following property assignment:
>> >>
>> >>     property = <0x12345678 'a' '\r' 100>;
>> >>
>> >> is equivalent to:
>> >>
>> >>     property = <0x12345678 0x00000061 0x0000000D 0x00000064>
>> >
>> > One tiny nit..
>> >
>> > [snip]
>> >> diff --git a/dtc-lexer.l b/dtc-lexer.l
>> >> index e866ea5..94151da 100644
>> >> --- a/dtc-lexer.l
>> >> +++ b/dtc-lexer.l
>> >> @@ -29,6 +29,7 @@ PROPNODECHAR        [a-zA-Z0-9,._+*#?@-]
>> >>  PATHCHAR     ({PROPNODECHAR}|[/])
>> >>  LABEL                [a-zA-Z_][a-zA-Z0-9_]*
>> >>  STRING               \"([^\\"]|\\.)*\"
>> >> +CHAR_LITERAL '[^']+'|'\\''
>> >
>> > I think this should be
>> >        '([^']|\\')*'
>> >
>> > On the same grounds I described before.  That does mean
>> > eval_char_literal() will have to cope with the case of ''.
>>
>> OK, I could also make it '([^']|\\')+'
>>
>> That way the empty character literal wouldn't be allowed and it still
>> allows all of the cases we care about.
>
> You could, but the same reasoning applies here:  An error message
> saying "empty character literal" is almost certainly more useful than
> "syntax error, Unexpected '" or whatever bison will give us on its
> own.

Good point, will do.

Thanks,
     Anton

> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>

Re: [PATCH v3 3/3] dtc: Support character literals in bytestrings

[Anton Staaf]

On Fri, Sep 9, 2011 at 12:01 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Thu, Sep 08, 2011 at 11:37:21PM -0700, Anton Staaf wrote:
>> On Thu, Sep 8, 2011 at 6:15 PM, David Gibson
>> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>> > On Thu, Sep 08, 2011 at 02:39:17PM -0700, Anton Staaf wrote:
>> >> With this patch the following property assignment:
>> >>
>> >>     property = ['a' 2b '\r'];
>> >>
>> >> is equivalent to:
>> >>
>> >>     property = [61 2b 0d];
>> >
>> > [snip]
>> >> +<BYTESTRING>{CHAR_LITERAL} {
>> >
>> > You should just be able to make the existing rule a <*> one,
>> > rather than having a separate rule for BYTESTRING context.
>>
>> I thought about that too, but I was concerned that it would then allow
>> character literals in properties outside of the cell list or
>> bytestring syntax.  Which I suppose is exactly what you had suggested
>> before now that I think about it.  And would end up being no more
>> ambiguous, and possibly more internally consistent.  I'll make the
>> change and add a test case that shows that functionality as well if
>> that makes sense to you.
>
> Well, it would allow character literals to be *lexed* in other places,
> but the parser would still reject them outside of the (so far)
> intended contexts without further changes.

True, and it simplifies the lexer.  OK, I'll make that change as well.

Thanks,
    Anton

> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>