[PATCH 0/2] Character literal parsing

[PATCH 0/2] Character literal parsing

[Anton Staaf]

    These patches add simple and escaped character literal parsing support
to the dct for bytestrings.  The first patch is very simple and provides
most of the functionality that is useful.  The second patch is of more
debatable value.  I've included it for completeness.

    Thanks,
        Anton

--

Anton Staaf (2):
  Add character literal parsing in bytestrings
  Add escaped character support to character literals

 Documentation/dts-format.txt |    5 +-
 Documentation/manual.txt     |    7 ++-
 data.c                       |   77 +--------------------------------
 dtc-lexer.l                  |   16 +++++++
 util.c                       |   98 ++++++++++++++++++++++++++++++++++++++++++
 util.h                       |    5 ++
 6 files changed, 127 insertions(+), 81 deletions(-)

-- 
1.7.3.1

[PATCH 1/2] Add character literal parsing in bytestrings

[Anton Staaf]

This adds support for parsing simple (non-escaped) 'x' character
literal syntax in bytestrings.  For example:

    property = ['a' 2b 'c'];

is equivalent to:

    property = [61 2b 62];

Signed-off-by: Anton Staaf <robotboy-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>
---
 Documentation/dts-format.txt |    5 +++--
 Documentation/manual.txt     |    3 ++-
 dtc-lexer.l                  |    8 ++++++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/dts-format.txt b/Documentation/dts-format.txt
index a655b87..d191eeb 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/Documentation/manual.txt b/Documentation/manual.txt
index f8a8a7b..f38a995 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -213,7 +213,8 @@ For example:
 
 By default, all numeric values are hexadecimal.  Alternate bases
 may be specified using a prefix "d#" for decimal, "b#" for binary,
-and "o#" for octal.
+and "o#" for octal.  Character literals are supported in
+byte sequences using the C language character literal syntax of 'a'.
 
 Strings support common escape sequences from C: "\n", "\t", "\r",
 "\(octal value)", "\x(hex value)".
diff --git a/dtc-lexer.l b/dtc-lexer.l
index e866ea5..1276c6f 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
@@ -128,6 +129,13 @@ static int pop_input_file(void);
 			return DT_BYTE;
 		}
 
+<BYTESTRING>{CHAR_LITERAL} {
+			DPRINT("Character literal: %s\n", yytext);
+			yylval.byte = yytext[1];
+			DPRINT("Byte: %02x\n", (int)yylval.byte);
+			return DT_BYTE;
+		}
+
 <BYTESTRING>"]"	{
 			DPRINT("/BYTESTRING\n");
 			BEGIN_DEFAULT();
-- 
1.7.3.1

[PATCH 2/2] Add escaped character support to character literals

[Anton Staaf]

This adds support for escaped characters in character literals using
'\r', '\012' and '\x4a' style escapes.

Signed-off-by: Anton Staaf <robotboy-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>
---
 Documentation/manual.txt |    4 +-
 data.c                   |   77 +-----------------------------------
 dtc-lexer.l              |    8 ++++
 util.c                   |   98 ++++++++++++++++++++++++++++++++++++++++++++++
 util.h                   |    5 ++
 5 files changed, 114 insertions(+), 78 deletions(-)

diff --git a/Documentation/manual.txt b/Documentation/manual.txt
index f38a995..34f88de 100644
--- a/Documentation/manual.txt
+++ b/Documentation/manual.txt
@@ -216,8 +216,8 @@ may be specified using a prefix "d#" for decimal, "b#" for binary,
 and "o#" for octal.  Character literals are supported in
 byte sequences using the C language character literal syntax of 'a'.
 
-Strings support common escape sequences from C: "\n", "\t", "\r",
-"\(octal value)", "\x(hex value)".
+Strings and byte sequences support common escape sequences from C:
+"\n", "\t", "\r", "\(octal value)", "\x(hex value)".
 
 
 4.3) Labels and References
diff --git a/data.c b/data.c
index fe555e8..471f6c3 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;
@@ -119,48 +85,7 @@ struct data data_copy_escape_string(const char *s, int len)
 			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;
-		}
+                q[d.len++] = get_escape_char(s, &i);
 	}
 
 	q[d.len++] = '\0';
diff --git a/dtc-lexer.l b/dtc-lexer.l
index 1276c6f..b6206ce 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -30,6 +30,7 @@ PATHCHAR	({PROPNODECHAR}|[/])
 LABEL		[a-zA-Z_][a-zA-Z0-9_]*
 STRING		\"([^\\"]|\\.)*\"
 CHAR_LITERAL	'[^\\']'
+CHAR_ESCAPED	'\\([^']+|')'
 WS		[[:space:]]
 COMMENT		"/*"([^*]|\*+[^*/])*\*+"/"
 LINECOMMENT	"//".*\n
@@ -136,6 +137,13 @@ static int pop_input_file(void);
 			return DT_BYTE;
 		}
 
+<BYTESTRING>{CHAR_ESCAPED} {
+			DPRINT("Character escaped literal: %s\n", yytext);
+			yylval.byte = get_escape_char_exact(yytext+1, yyleng-2);
+			DPRINT("Byte: %02x\n", (int)yylval.byte);
+			return DT_BYTE;
+		}
+
 <BYTESTRING>"]"	{
 			DPRINT("/BYTESTRING\n");
 			BEGIN_DEFAULT();
diff --git a/util.c b/util.c
index d7ac27d..2c58007 100644
--- a/util.c
+++ b/util.c
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <string.h>
+#include <assert.h>
 
 #include "util.h"
 
@@ -57,3 +58,100 @@ char *join_path(const char *path, const char *name)
 	memcpy(str+lenp, name, lenn+1);
 	return str;
 }
+
+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;
+}
+
+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--;
+		val = get_oct_char(s, &j);
+		break;
+        case 'x':
+		val = get_hex_char(s, &j);
+		break;
+        default:
+		val = c;
+        }
+
+	(*i) = j;
+	return val;
+}
+
+char get_escape_char_exact(const char *s, int len)
+{
+	int	j = 1; //skip intial "\"
+        char	c = get_escape_char(s, &j);
+
+	if (j != len)
+		die("Extra characters at end of character literal '%s' "
+                    "(%d != %d)\n", s, j, len);
+
+	return c;
+}
diff --git a/util.h b/util.h
index 9cead84..c60ad9d 100644
--- a/util.h
+++ b/util.h
@@ -53,4 +53,9 @@ static inline void *xrealloc(void *p, size_t len)
 extern char *xstrdup(const char *s);
 extern char *join_path(const char *path, const char *name);
 
+extern char get_oct_char(const char *s, int *i);
+extern char get_hex_char(const char *s, int *i);
+extern char get_escape_char(const char *s, int *i);
+extern char get_escape_char_exact(const char *s, int len);
+
 #endif /* _UTIL_H */
-- 
1.7.3.1

Re: [PATCH 0/2] Character literal parsing

[David Brown]

On Thu, Jun 23 2011, Anton Staaf wrote:

> These patches add simple and escaped character literal parsing support
> to the dct for bytestrings.  The first patch is very simple and provides
> most of the functionality that is useful.  The second patch is of more
> debatable value.  I've included it for completeness.

Does this mean that the dtc inside the kernel is going to be the
required tool to use to build device trees?  This change doesn't change
the DTB format, so it's as much of a concern, but was wondering if we're
intending to keep things compatible.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: [PATCH 0/2] Character literal parsing

[Anton Staaf]

[-- Attachment #1.1: Type: text/plain, Size: 847 bytes --]

On Fri, Jun 24, 2011 at 1:02 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:

>
> Does this mean that the dtc inside the kernel is going to be the
> required tool to use to build device trees?  This change doesn't change
> the DTB format, so it's as much of a concern, but was wondering if we're
> intending to keep things compatible.
>

To be honest, I don't know enough to say either way.  I am using the
character
literals in a device tree that is used to configure a single firmware image
for
multiple boards.  That device tree is not currently passed on to the kernel.

Your question makes me think that there are two device tree compilers that I
should be paying attention to, is that the case?  Or was it a more general
comment about diverging from a historic syntax for device tree source files?

Thanks,
    Anton

[-- Attachment #1.2: Type: text/html, Size: 1347 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/2] Character literal parsing

[David Brown]

On Fri, Jun 24 2011, Anton Staaf wrote:

> On Fri, Jun 24, 2011 at 1:02 PM, David Brown <davidb@codeaurora.org> wrote:
>
>
>     Does this mean that the dtc inside the kernel is going to be the
>     required tool to use to build device trees?  This change doesn't change
>     the DTB format, so it's as much of a concern, but was wondering if we're
>     intending to keep things compatible.
>
>
> To be honest, I don't know enough to say either way.  I am using the character
> literals in a device tree that is used to configure a single firmware image for
> multiple boards.  That device tree is not currently passed on to the kernel.
>
> Your question makes me think that there are two device tree compilers that I
> should be paying attention to, is that the case?  Or was it a more general
> comment about diverging from a historic syntax for device tree source files?

Both, really.  There is a dtc at
git://git.kernel.org/pub/scm/linux/kernel/git/galak/dtc.git but it seems
older than the one in the kernel.

Also, the dts form is defined in the ePAPR documents, and this would be
a (minor) divergence from that.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/2] Character literal parsing

[Jon Loeliger]

> Your question makes me think that there are two device tree compilers

There is one device tree compiler.  The main "upstream" repository
is here:

    git://git.jdl.com/software/dtc.git

There is a snapshot of that which is occasionaly brought
into the kernel repository.

jdl

Re: [PATCH 0/2] Character literal parsing

[Anton Staaf]

[-- Attachment #1.1: Type: text/plain, Size: 433 bytes --]

On Fri, Jun 24, 2011 at 2:17 PM, Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org> wrote:

> > Your question makes me think that there are two device tree compilers
>
> There is one device tree compiler.  The main "upstream" repository
> is here:
>
>    git://git.jdl.com/software/dtc.git
>
> There is a snapshot of that which is occasionaly brought
> into the kernel repository.
>

Excellent, thank you for the clarification.

-Anton

[-- Attachment #1.2: Type: text/html, Size: 860 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/2] Character literal parsing

[Anton Staaf]

[-- Attachment #1.1: Type: text/plain, Size: 648 bytes --]

On Fri, Jun 24, 2011 at 2:11 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:

>
> Also, the dts form is defined in the ePAPR documents, and this would be
> a (minor) divergence from that.
>
>
Thanks for the pointer, I had not read the ePAPR docs, I scanned them (will
read in more depth later) and didn't find any surprises.

So I guess the question is, are we comfortable with a divergence from the
spec there, and if so, are character literals generally useful enough to be
considered for inclusion in the canonical dtc?  Jon, do you have a feeling
one way or the other about character literal support?

Thanks,
    Anton

[-- Attachment #1.2: Type: text/html, Size: 1077 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/2] Character literal parsing

[Grant Likely]

On Fri, Jun 24, 2011 at 3:11 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On Fri, Jun 24 2011, Anton Staaf wrote:
>
>> On Fri, Jun 24, 2011 at 1:02 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>
>>
>>     Does this mean that the dtc inside the kernel is going to be the
>>     required tool to use to build device trees?  This change doesn't change
>>     the DTB format, so it's as much of a concern, but was wondering if we're
>>     intending to keep things compatible.
>>
>>
>> To be honest, I don't know enough to say either way.  I am using the character
>> literals in a device tree that is used to configure a single firmware image for
>> multiple boards.  That device tree is not currently passed on to the kernel.
>>
>> Your question makes me think that there are two device tree compilers that I
>> should be paying attention to, is that the case?  Or was it a more general
>> comment about diverging from a historic syntax for device tree source files?
>
> Both, really.  There is a dtc at
> git://git.kernel.org/pub/scm/linux/kernel/git/galak/dtc.git but it seems
> older than the one in the kernel.

The kernel one is simply a copy of the upstream dtc.  You should craft
your patches against:

git://git.jdl.com/software/dtc.git

>
> Also, the dts form is defined in the ePAPR documents, and this would be
> a (minor) divergence from that.

dts is not set in stone, and is certainly subject to enhancements
providing it doesn't break existing users.

g.

Re: [PATCH 0/2] Character literal parsing

[Anton Staaf]

[-- Attachment #1.1: Type: text/plain, Size: 2129 bytes --]

On Fri, Jun 24, 2011 at 2:34 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>wrote:

> On Fri, Jun 24, 2011 at 3:11 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> wrote:
> > On Fri, Jun 24 2011, Anton Staaf wrote:
> >
> >> On Fri, Jun 24, 2011 at 1:02 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> wrote:
> >>
> >>
> >>     Does this mean that the dtc inside the kernel is going to be the
> >>     required tool to use to build device trees?  This change doesn't
> change
> >>     the DTB format, so it's as much of a concern, but was wondering if
> we're
> >>     intending to keep things compatible.
> >>
> >>
> >> To be honest, I don't know enough to say either way.  I am using the
> character
> >> literals in a device tree that is used to configure a single firmware
> image for
> >> multiple boards.  That device tree is not currently passed on to the
> kernel.
> >>
> >> Your question makes me think that there are two device tree compilers
> that I
> >> should be paying attention to, is that the case?  Or was it a more
> general
> >> comment about diverging from a historic syntax for device tree source
> files?
> >
> > Both, really.  There is a dtc at
> > git://git.kernel.org/pub/scm/linux/kernel/git/galak/dtc.git but it seems
> > older than the one in the kernel.
>
> The kernel one is simply a copy of the upstream dtc.  You should craft
> your patches against:
>
> git://git.jdl.com/software/dtc.git
>
> >
> > Also, the dts form is defined in the ePAPR documents, and this would be
> > a (minor) divergence from that.
>
> dts is not set in stone, and is certainly subject to enhancements
> providing it doesn't break existing users.
>
>
Adding character literals does not (as far as I can tell) conflict with any
existing syntax.  All exiting dts files should compile to the exact same
blobs with my patches.  And of coarse, all of the existing test cases pass
cleanly.  It would also be a good idea for me to add test cases for
character literals.  I will do that and update the patch set if we decide to
accept the change to the syntax.

-Anton

[-- Attachment #1.2: Type: text/html, Size: 3078 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/2] Character literal parsing

[Anton Staaf]

[-- Attachment #1.1: Type: text/plain, Size: 2457 bytes --]

Jon, what are your feelings on the proposed patch?  I'd love to get an
indication either way, should I continue to pursue this?

Thanks,
    Anton

On Fri, Jun 24, 2011 at 2:47 PM, Anton Staaf <robotboy-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:

>
>
> On Fri, Jun 24, 2011 at 2:34 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>wrote:
>
>> On Fri, Jun 24, 2011 at 3:11 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> wrote:
>> > On Fri, Jun 24 2011, Anton Staaf wrote:
>> >
>> >> On Fri, Jun 24, 2011 at 1:02 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> wrote:
>> >>
>> >>
>> >>     Does this mean that the dtc inside the kernel is going to be the
>> >>     required tool to use to build device trees?  This change doesn't
>> change
>> >>     the DTB format, so it's as much of a concern, but was wondering if
>> we're
>> >>     intending to keep things compatible.
>> >>
>> >>
>> >> To be honest, I don't know enough to say either way.  I am using the
>> character
>> >> literals in a device tree that is used to configure a single firmware
>> image for
>> >> multiple boards.  That device tree is not currently passed on to the
>> kernel.
>> >>
>> >> Your question makes me think that there are two device tree compilers
>> that I
>> >> should be paying attention to, is that the case?  Or was it a more
>> general
>> >> comment about diverging from a historic syntax for device tree source
>> files?
>> >
>> > Both, really.  There is a dtc at
>> > git://git.kernel.org/pub/scm/linux/kernel/git/galak/dtc.git but it
>> seems
>> > older than the one in the kernel.
>>
>> The kernel one is simply a copy of the upstream dtc.  You should craft
>> your patches against:
>>
>> git://git.jdl.com/software/dtc.git
>>
>> >
>> > Also, the dts form is defined in the ePAPR documents, and this would be
>> > a (minor) divergence from that.
>>
>> dts is not set in stone, and is certainly subject to enhancements
>> providing it doesn't break existing users.
>>
>>
> Adding character literals does not (as far as I can tell) conflict with any
> existing syntax.  All exiting dts files should compile to the exact same
> blobs with my patches.  And of coarse, all of the existing test cases pass
> cleanly.  It would also be a good idea for me to add test cases for
> character literals.  I will do that and update the patch set if we decide to
> accept the change to the syntax.
>
> -Anton
>
>

[-- Attachment #1.2: Type: text/html, Size: 3721 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/2] Add character literal parsing in bytestrings

[David Gibson]

On Thu, Jun 23, 2011 at 04:20:38PM -0700, Anton Staaf wrote:
> This adds support for parsing simple (non-escaped) 'x' character
> literal syntax in bytestrings.  For example:
> 
>     property = ['a' 2b 'c'];
> 
> is equivalent to:
> 
>     property = [61 2b 62];

Hrm.  I like the idea of being able to encode character literals.
However I'm dubious as to whether the bytestring syntax is the right
place to encode them.

Bytestrings are quite lexically strange, they are quite different from
the < ... > cell syntax: the things inside default to hex, and spacing
is irrelevant ([abcd] is equivalent to [ab cd], [a bc] is a syntax
error and *not* equivalent to [0a bc]).  This makes me worry about
possible ambiguities or other parsing problems if we put something
other than exactly 2 digit hex bytes in there - not that I can see any
definite ambiguities in this proposal.

I have for some time been intending to introduce some variant of the
< > syntax which allows for different sized entries (1, 2 or 8 bytes
instead of the default 4).  I just haven't thought of a nice syntax
for it yet.  Character literals would fit just fine into that scheme.

The other possibility would be to allow something like 'abcd' without
any special context to be a non-NULL-terminated "string".  I'm also a
bit dubious about that on the "be like C" principle, which has served
us pretty well in the past.

-- 
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 0/2] Character literal parsing

[David Gibson]

On Fri, Jun 24, 2011 at 01:22:13PM -0700, Anton Staaf wrote:
> On Fri, Jun 24, 2011 at 1:02 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> 
> >
> > Does this mean that the dtc inside the kernel is going to be the
> > required tool to use to build device trees?  This change doesn't change
> > the DTB format, so it's as much of a concern, but was wondering if we're
> > intending to keep things compatible.
> >
> 
> To be honest, I don't know enough to say either way.  I am using the
> character literals in a device tree that is used to configure a
> single firmware image for multiple boards.  That device tree is not
> currently passed on to the kernel.
> 
> Your question makes me think that there are two device tree compilers that I
> should be paying attention to, is that the case?  Or was it a more general
> comment about diverging from a historic syntax for device tree
> source files?

Oh, yes, forgot to mention that.

If you want to make extensions to dtc, you should do it to the
upstream dtc at git://git.jdl.com/software/dtc.git.  The in-kernel
version is just a irregularly updated snapshot of that one.

-- 
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 0/2] Character literal parsing

[David Gibson]

On Fri, Jun 24, 2011 at 02:11:08PM -0700, David Brown wrote:
> On Fri, Jun 24 2011, Anton Staaf wrote:
> 
> > On Fri, Jun 24, 2011 at 1:02 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> >
> >
> >     Does this mean that the dtc inside the kernel is going to be the
> >     required tool to use to build device trees?  This change doesn't change
> >     the DTB format, so it's as much of a concern, but was wondering if we're
> >     intending to keep things compatible.
> >
> >
> > To be honest, I don't know enough to say either way.  I am using the character
> > literals in a device tree that is used to configure a single firmware image for
> > multiple boards.  That device tree is not currently passed on to the kernel.
> >
> > Your question makes me think that there are two device tree compilers that I
> > should be paying attention to, is that the case?  Or was it a more general
> > comment about diverging from a historic syntax for device tree source files?
> 
> Both, really.  There is a dtc at
> git://git.kernel.org/pub/scm/linux/kernel/git/galak/dtc.git but it seems
> older than the one in the kernel.
> 
> Also, the dts form is defined in the ePAPR documents, and this would be
> a (minor) divergence from that.

That's fine as long as the change is backwards compatible.

-- 
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 0/2] Character literal parsing

[Anton Staaf]

On Wed, Jul 20, 2011 at 6:42 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Fri, Jun 24, 2011 at 01:22:13PM -0700, Anton Staaf wrote:
>> On Fri, Jun 24, 2011 at 1:02 PM, David Brown <davidb-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>
>> >
>> > Does this mean that the dtc inside the kernel is going to be the
>> > required tool to use to build device trees?  This change doesn't change
>> > the DTB format, so it's as much of a concern, but was wondering if we're
>> > intending to keep things compatible.
>> >
>>
>> To be honest, I don't know enough to say either way.  I am using the
>> character literals in a device tree that is used to configure a
>> single firmware image for multiple boards.  That device tree is not
>> currently passed on to the kernel.
>>
>> Your question makes me think that there are two device tree compilers that I
>> should be paying attention to, is that the case?  Or was it a more general
>> comment about diverging from a historic syntax for device tree
>> source files?
>
> Oh, yes, forgot to mention that.
>
> If you want to make extensions to dtc, you should do it to the
> upstream dtc at git://git.jdl.com/software/dtc.git.  The in-kernel
> version is just a irregularly updated snapshot of that one.

Yup, that's the repository I formed my patches against.  My
understanding was that this was the correct mailing list to send such
patches to.  But if there is another way to submit patches to the
upstream dtc repository please let me know.

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 1/2] Add character literal parsing in bytestrings

[Anton Staaf]

On Wed, Jul 20, 2011 at 6:40 AM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Thu, Jun 23, 2011 at 04:20:38PM -0700, Anton Staaf wrote:
>> This adds support for parsing simple (non-escaped) 'x' character
>> literal syntax in bytestrings.  For example:
>>
>>     property = ['a' 2b 'c'];
>>
>> is equivalent to:
>>
>>     property = [61 2b 62];
>
> Hrm.  I like the idea of being able to encode character literals.
> However I'm dubious as to whether the bytestring syntax is the right
> place to encode them.
>
> Bytestrings are quite lexically strange, they are quite different from
> the < ... > cell syntax: the things inside default to hex, and spacing
> is irrelevant ([abcd] is equivalent to [ab cd], [a bc] is a syntax
> error and *not* equivalent to [0a bc]).  This makes me worry about
> possible ambiguities or other parsing problems if we put something
> other than exactly 2 digit hex bytes in there - not that I can see any
> definite ambiguities in this proposal.

As you point out below, the < ... > syntax doesn't permit byte values
(a cell is 32 bits).  So using the cell list syntax would create a lot
of wasted space.  Especially in my use where I need to create four 128
byte tables for keyboard scan code mapping.  It would end up wasting
>1KB.  Adding cell size control syntax would certainly solve that
problem.  Is this something your interested in pursuing at this time,
I'd be happy to help with that instead of continuing to push this.

Alternatively, I think it is clear that there are no problems parsing
out the character literals.  Mainly because the ' character is unique
and will never otherwise occur as a character in a byte literal
declaration.  The occurrence or lack there of of white space should
also not be a problem, since the character literal parsing is of a
fixed length, thus there is no possibility for an ambiguous use such
as ' ab '.  Also, the invalid use [a bc] is still invalid with
character literals added, for example [a 'b'] or [a'b'] are both
invalid because the existing bytestring regex only matches two hex
characters in a row, and the new character literal regex only matches
a single character bounded by single quotes.  So neither regex will
match the lone a character and parsing will fail there.

> I have for some time been intending to introduce some variant of the
> < > syntax which allows for different sized entries (1, 2 or 8 bytes
> instead of the default 4).  I just haven't thought of a nice syntax
> for it yet.  Character literals would fit just fine into that scheme.
>
> The other possibility would be to allow something like 'abcd' without
> any special context to be a non-NULL-terminated "string".  I'm also a
> bit dubious about that on the "be like C" principle, which has served
> us pretty well in the past.

Yes, I would also shy away from such a construct.  I think it violates
the principal of least surprise.  Though admittedly, the byte string
syntax does that already...

Thanks,
    Anton

Re: [PATCH 0/2] Character literal parsing

[Jon Loeliger]

> >
> > If you want to make extensions to dtc, you should do it to the
> > upstream dtc at git://git.jdl.com/software/dtc.git. =A0The in-kernel
> > version is just a irregularly updated snapshot of that one.
> 
> Yup, that's the repository I formed my patches against.  My
> understanding was that this was the correct mailing list to send such
> patches to.  But if there is another way to submit patches to the
> upstream dtc repository please let me know.

This is the correct list for DTC patches.

jdl

Re: [PATCH 1/2] Add character literal parsing in bytestrings

[David Gibson]

On Wed, Jul 20, 2011 at 09:50:43AM -0700, Anton Staaf wrote:
> On Wed, Jul 20, 2011 at 6:40 AM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Thu, Jun 23, 2011 at 04:20:38PM -0700, Anton Staaf wrote:
> >> This adds support for parsing simple (non-escaped) 'x' character
> >> literal syntax in bytestrings.  For example:
> >>
> >>     property = ['a' 2b 'c'];
> >>
> >> is equivalent to:
> >>
> >>     property = [61 2b 62];
> >
> > Hrm.  I like the idea of being able to encode character literals.
> > However I'm dubious as to whether the bytestring syntax is the right
> > place to encode them.
> >
> > Bytestrings are quite lexically strange, they are quite different from
> > the < ... > cell syntax: the things inside default to hex, and spacing
> > is irrelevant ([abcd] is equivalent to [ab cd], [a bc] is a syntax
> > error and *not* equivalent to [0a bc]).  This makes me worry about
> > possible ambiguities or other parsing problems if we put something
> > other than exactly 2 digit hex bytes in there - not that I can see any
> > definite ambiguities in this proposal.
> 
> As you point out below, the < ... > syntax doesn't permit byte values
> (a cell is 32 bits).  So using the cell list syntax would create a lot
> of wasted space.  Especially in my use where I need to create four 128
> byte tables for keyboard scan code mapping.  It would end up wasting
> >1KB.

I certainly wasn't suggesting using padding.  Apart from the wasted
space, it wouldn't let you use it for an already defined binding which
lacks the padding.

>  Adding cell size control syntax would certainly solve that
> problem.  Is this something your interested in pursuing at this time,
> I'd be happy to help with that instead of continuing to push this.

Well, to be honest I'd love to have this syntax several years ago :).
The implementation should be almost trivial, really the only stumbling
block is finding a syntax which is unambiguous, won't cause parsing
oddities and obeys the principle least surprise as best we can.

> Alternatively, I think it is clear that there are no problems parsing
> out the character literals.  Mainly because the ' character is unique
> and will never otherwise occur as a character in a byte literal
> declaration.  The occurrence or lack there of of white space should
> also not be a problem, since the character literal parsing is of a
> fixed length, thus there is no possibility for an ambiguous use such
> as ' ab '.  Also, the invalid use [a bc] is still invalid with
> character literals added, for example [a 'b'] or [a'b'] are both
> invalid because the existing bytestring regex only matches two hex
> characters in a row, and the new character literal regex only matches
> a single character bounded by single quotes.  So neither regex will
> match the lone a character and parsing will fail there.

That's true.  Consider me about 40% persuaded :).

Ok, here's what I suggest.  For now, can you create a patch which
recognizes the character construct syntax in the lexer (including
escapes), and allows its use in cell context.  That won't actually do
what you want, but it gets a fair chunk of the code in a testable,
upstreamable form without making syntax changes I'm uncomfortable
with.

While we're getting that merged we can debate which/how to proceed
with either variable size cell syntax or allowing the character
literals in 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 1/2] Add character literal parsing in bytestrings

[Jon Loeliger]

> 
> Well, to be honest I'd love to have this syntax several years ago :).
>
> [...]
>
> Ok, here's what I suggest.  For now, can you create a patch which
> recognizes the character construct syntax in the lexer (including
> escapes), and allows its use in cell context.  That won't actually do
> what you want, but it gets a fair chunk of the code in a testable,
> upstreamable form without making syntax changes I'm uncomfortable
> with.
> 
> While we're getting that merged we can debate which/how to proceed
> with either variable size cell syntax or allowing the character
> literals in bytestring context.

Is it worth investigating any of the work I did down this line
several years ago?  It's all still in the "test" branch if so.

jdl

Re: [PATCH 1/2] Add character literal parsing in bytestrings

[David Gibson]

On Thu, Jul 21, 2011 at 07:41:57AM -0500, Jon Loeliger wrote:
> > 
> > Well, to be honest I'd love to have this syntax several years ago :).
> >
> > [...]
> >
> > Ok, here's what I suggest.  For now, can you create a patch which
> > recognizes the character construct syntax in the lexer (including
> > escapes), and allows its use in cell context.  That won't actually do
> > what you want, but it gets a fair chunk of the code in a testable,
> > upstreamable form without making syntax changes I'm uncomfortable
> > with.
> > 
> > While we're getting that merged we can debate which/how to proceed
> > with either variable size cell syntax or allowing the character
> > literals in bytestring context.
> 
> Is it worth investigating any of the work I did down this line
> several years ago?  It's all still in the "test" branch if so.

Well, it might be worth re-examining that in any case, but as far as I
can recall or tell from a quick look at the testing branch, everything
in there is more or less orthogonal to anything suggested in this
thread.

-- 
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