[0/10] dtc: Assorted parsing and error reporting cleanups

[0/10] dtc: Assorted parsing and error reporting cleanups

[David Gibson]

This series consists of a number of small fixes and cleanups, mostly
to error reporting in the lexer and parser.  I hit these as minor
annoyances while experimenting with improved expression and schema
support.

I'm sending these out for some review, before committing them to the
main branch.

These patches are also in the 'errors' branch on the repository - note
that I'm treating that as an experimental branch, and therefore
subject to rebasing at whim.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/10] Remove unused srcpos_warn() function

[David Gibson]

This function has no users, and we can replace it more generally later.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 srcpos.c | 17 -----------------
 srcpos.h |  2 --
 2 files changed, 19 deletions(-)

diff --git a/srcpos.c b/srcpos.c
index 294568b..06ef06d 100644
--- a/srcpos.c
+++ b/srcpos.c
@@ -312,23 +312,6 @@ srcpos_error(struct srcpos *pos, char const *fmt, ...)
 	va_end(va);
 }
 
-
-void
-srcpos_warn(struct srcpos *pos, char const *fmt, ...)
-{
-	const char *srcstr;
-	va_list va;
-	va_start(va, fmt);
-
-	srcstr = srcpos_string(pos);
-
-	fprintf(stderr, "Warning: %s ", srcstr);
-	vfprintf(stderr, fmt, va);
-	fprintf(stderr, "\n");
-
-	va_end(va);
-}
-
 void srcpos_set_line(char *f, int l)
 {
 	current_srcfile->name = f;
diff --git a/srcpos.h b/srcpos.h
index 57dfa0f..7f0eaea 100644
--- a/srcpos.h
+++ b/srcpos.h
@@ -111,8 +111,6 @@ extern void srcpos_verror(struct srcpos *pos, char const *, va_list va)
      __attribute__((format(printf, 2, 0)));
 extern void srcpos_error(struct srcpos *pos, char const *, ...)
      __attribute__((format(printf, 2, 3)));
-extern void srcpos_warn(struct srcpos *pos, char const *, ...)
-     __attribute__((format(printf, 2, 3)));
 
 extern void srcpos_set_line(char *f, int l);
 
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/10] Fix typo in type of srcpos_verror() et al.

[David Gibson]

The srcpos_verror() and srcpos_error() functions declare the format
string as 'char const *' instead of 'const char *'.  Fix it.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 srcpos.c | 4 ++--
 srcpos.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/srcpos.c b/srcpos.c
index 06ef06d..71a73b8 100644
--- a/srcpos.c
+++ b/srcpos.c
@@ -291,7 +291,7 @@ srcpos_string(struct srcpos *pos)
 }
 
 void
-srcpos_verror(struct srcpos *pos, char const *fmt, va_list va)
+srcpos_verror(struct srcpos *pos, const char *fmt, va_list va)
 {
        const char *srcstr;
 
@@ -303,7 +303,7 @@ srcpos_verror(struct srcpos *pos, char const *fmt, va_list va)
 }
 
 void
-srcpos_error(struct srcpos *pos, char const *fmt, ...)
+srcpos_error(struct srcpos *pos, const char *fmt, ...)
 {
 	va_list va;
 
diff --git a/srcpos.h b/srcpos.h
index 7f0eaea..46ea08a 100644
--- a/srcpos.h
+++ b/srcpos.h
@@ -107,9 +107,9 @@ extern struct srcpos *srcpos_copy(struct srcpos *pos);
 extern char *srcpos_string(struct srcpos *pos);
 extern void srcpos_dump(struct srcpos *pos);
 
-extern void srcpos_verror(struct srcpos *pos, char const *, va_list va)
+extern void srcpos_verror(struct srcpos *pos, const char *, va_list va)
      __attribute__((format(printf, 2, 0)));
-extern void srcpos_error(struct srcpos *pos, char const *, ...)
+extern void srcpos_error(struct srcpos *pos, const char *, ...)
      __attribute__((format(printf, 2, 3)));
 
 extern void srcpos_set_line(char *f, int l);
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/10] Fix indentation of srcpos_verror()

[David Gibson]

Somehow this function ended up with a 7 space indent, instead of the usual
8 space (1 tab) indent.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 srcpos.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/srcpos.c b/srcpos.c
index 71a73b8..48059aa 100644
--- a/srcpos.c
+++ b/srcpos.c
@@ -293,13 +293,13 @@ srcpos_string(struct srcpos *pos)
 void
 srcpos_verror(struct srcpos *pos, const char *fmt, va_list va)
 {
-       const char *srcstr;
+	const char *srcstr;
 
-       srcstr = srcpos_string(pos);
+	srcstr = srcpos_string(pos);
 
-       fprintf(stderr, "Error: %s ", srcstr);
-       vfprintf(stderr, fmt, va);
-       fprintf(stderr, "\n");
+	fprintf(stderr, "Error: %s ", srcstr);
+	vfprintf(stderr, fmt, va);
+	fprintf(stderr, "\n");
 }
 
 void
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/10] Fix memory leak in srcpos_verror()

[David Gibson]

Since dtc runs are short, we don't care that much about memory leaks.
Still, leaking the source position string every time we print an error
messages is pretty nasty.  Fix it.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 srcpos.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/srcpos.c b/srcpos.c
index 48059aa..6705fed 100644
--- a/srcpos.c
+++ b/srcpos.c
@@ -293,13 +293,15 @@ srcpos_string(struct srcpos *pos)
 void
 srcpos_verror(struct srcpos *pos, const char *fmt, va_list va)
 {
-	const char *srcstr;
+	char *srcstr;
 
 	srcstr = srcpos_string(pos);
 
 	fprintf(stderr, "Error: %s ", srcstr);
 	vfprintf(stderr, fmt, va);
 	fprintf(stderr, "\n");
+
+	free(srcstr);
 }
 
 void
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/10] Make srcpos_{v,}error() more widely useful

[David Gibson]

Allow them to take a prefix argument giving the general type of error,
which will be useful in future.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 dtc-parser.y |  2 +-
 srcpos.c     | 12 ++++++------
 srcpos.h     | 10 ++++++----
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 4864631..2bcef1b 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -475,7 +475,7 @@ void print_error(char const *fmt, ...)
 	va_list va;
 
 	va_start(va, fmt);
-	srcpos_verror(&yylloc, fmt, va);
+	srcpos_verror(&yylloc, "Error", fmt, va);
 	va_end(va);
 
 	treesource_error = true;
diff --git a/srcpos.c b/srcpos.c
index 6705fed..3999675 100644
--- a/srcpos.c
+++ b/srcpos.c
@@ -290,27 +290,27 @@ srcpos_string(struct srcpos *pos)
 	return pos_str;
 }
 
-void
-srcpos_verror(struct srcpos *pos, const char *fmt, va_list va)
+void srcpos_verror(struct srcpos *pos, const char *prefix,
+		   const char *fmt, va_list va)
 {
 	char *srcstr;
 
 	srcstr = srcpos_string(pos);
 
-	fprintf(stderr, "Error: %s ", srcstr);
+	fprintf(stderr, "%s: %s ", prefix, srcstr);
 	vfprintf(stderr, fmt, va);
 	fprintf(stderr, "\n");
 
 	free(srcstr);
 }
 
-void
-srcpos_error(struct srcpos *pos, const char *fmt, ...)
+void srcpos_error(struct srcpos *pos, const char *prefix,
+		  const char *fmt, ...)
 {
 	va_list va;
 
 	va_start(va, fmt);
-	srcpos_verror(pos, fmt, va);
+	srcpos_verror(pos, prefix, fmt, va);
 	va_end(va);
 }
 
diff --git a/srcpos.h b/srcpos.h
index 46ea08a..f81827b 100644
--- a/srcpos.h
+++ b/srcpos.h
@@ -107,10 +107,12 @@ extern struct srcpos *srcpos_copy(struct srcpos *pos);
 extern char *srcpos_string(struct srcpos *pos);
 extern void srcpos_dump(struct srcpos *pos);
 
-extern void srcpos_verror(struct srcpos *pos, const char *, va_list va)
-     __attribute__((format(printf, 2, 0)));
-extern void srcpos_error(struct srcpos *pos, const char *, ...)
-     __attribute__((format(printf, 2, 3)));
+extern void srcpos_verror(struct srcpos *pos, const char *prefix,
+			  const char *fmt, va_list va)
+	__attribute__((format(printf, 3, 0)));
+extern void srcpos_error(struct srcpos *pos, const char *prefix,
+			 const char *fmt, ...)
+	__attribute__((format(printf, 3, 4)));
 
 extern void srcpos_set_line(char *f, int l);
 
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/10] Move integer literal processing back to the lexer

[David Gibson]

At the moment integer literals are passed from the lexer to the parser as
a string, where it's evaluated into an integer by eval_literal().  That
strange approach happened because we needed to know whether we were
processing dts-v0 or dts-v1 - only known at the parser level - to know
how to interpret the literal properly.

dts-v0 support has been gone for some time now, and the base and bits
parameters to eval_literal() are essentially useless.

So, clean things up by moving the literal interpretation back to the lexer.
This also introduces a new lexical_error() function to report malformed
literals and set the treesource_error flag so that they'll cause a parse
failure at the top level.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 dtc-lexer.l  | 30 ++++++++++++++++++++++++++++--
 dtc-parser.y | 42 ++++++++++--------------------------------
 2 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/dtc-lexer.l b/dtc-lexer.l
index 0cd7e67..ba5d150 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -40,6 +40,7 @@ LINECOMMENT	"//".*\n
 #include "dtc-parser.tab.h"
 
 YYLTYPE yylloc;
+extern bool treesource_error;
 
 /* CAUTION: this will stop working if we ever use yyless() or yyunput() */
 #define	YY_USER_ACTION \
@@ -62,6 +63,7 @@ static int dts_version = 1;
 
 static void push_input_file(const char *filename);
 static bool pop_input_file(void);
+static void lexical_error(const char *fmt, ...);
 %}
 
 %%
@@ -146,8 +148,21 @@ static bool pop_input_file(void);
 		}
 
 <V1>([0-9]+|0[xX][0-9a-fA-F]+)(U|L|UL|LL|ULL)? {
-			yylval.literal = xstrdup(yytext);
-			DPRINT("Literal: '%s'\n", yylval.literal);
+			char *e;
+			DPRINT("Integer Literal: '%s'\n", yytext);
+
+			errno = 0;
+			yylval.integer = strtoull(yytext, &e, 0);
+
+			assert(!(*e) || !e[strspn(e, "UL")]);
+
+			if (errno == ERANGE)
+				lexical_error("Integer literal '%s' out of range",
+					      yytext);
+			else
+				/* ERANGE is the only strtoull error triggerable
+				 *  by strings matching the pattern */
+				assert(errno == 0);
 			return DT_LITERAL;
 		}
 
@@ -248,3 +263,14 @@ static bool pop_input_file(void)
 
 	return true;
 }
+
+static void lexical_error(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	srcpos_verror(&yylloc, "Lexical error", fmt, ap);
+	va_end(ap);
+
+	treesource_error = true;
+}
diff --git a/dtc-parser.y b/dtc-parser.y
index 2bcef1b..0ce0815 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -33,7 +33,6 @@ extern void yyerror(char const *s);
 extern struct boot_info *the_boot_info;
 extern bool treesource_error;
 
-static unsigned long long eval_literal(const char *s, int base, int bits);
 static unsigned char eval_char_literal(const char *s);
 %}
 
@@ -65,7 +64,7 @@ static unsigned char eval_char_literal(const char *s);
 %token DT_DEL_PROP
 %token DT_DEL_NODE
 %token <propnodename> DT_PROPNODENAME
-%token <literal> DT_LITERAL
+%token <integer> DT_LITERAL
 %token <literal> DT_CHAR_LITERAL
 %token <cbase> DT_BASE
 %token <byte> DT_BYTE
@@ -274,18 +273,20 @@ propdataprefix:
 arrayprefix:
 	DT_BITS DT_LITERAL '<'
 		{
-			$$.data = empty_data;
-			$$.bits = eval_literal($2, 0, 7);
+			unsigned long long bits;
+
+			bits = $2;
 
-			if (($$.bits !=  8) &&
-			    ($$.bits != 16) &&
-			    ($$.bits != 32) &&
-			    ($$.bits != 64))
+			if ((bits !=  8) && (bits != 16) &&
+			    (bits != 32) && (bits != 64))
 			{
 				print_error("Only 8, 16, 32 and 64-bit elements"
 					    " are currently supported");
-				$$.bits = 32;
+				bits = 32;
 			}
+
+			$$.data = empty_data;
+			$$.bits = bits;
 		}
 	| '<'
 		{
@@ -334,9 +335,6 @@ arrayprefix:
 
 integer_prim:
 	  DT_LITERAL
-		{
-			$$ = eval_literal($1, 0, 64);
-		}
 	| DT_CHAR_LITERAL
 		{
 			$$ = eval_char_literal($1);
@@ -485,26 +483,6 @@ void yyerror(char const *s) {
 	print_error("%s", s);
 }
 
-static unsigned long long eval_literal(const char *s, int base, int bits)
-{
-	unsigned long long val;
-	char *e;
-
-	errno = 0;
-	val = strtoull(s, &e, base);
-	if (*e) {
-		size_t uls = strspn(e, "UL");
-		if (e[uls])
-			print_error("bad characters in literal");
-	}
-	if ((errno == ERANGE)
-		 || ((bits < 64) && (val >= (1ULL << bits))))
-		print_error("literal out of range");
-	else if (errno != 0)
-		print_error("bad literal");
-	return val;
-}
-
 static unsigned char eval_char_literal(const char *s)
 {
 	int i = 1;
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/10] Move character literal processing to the lexer

[David Gibson]

To match the processing of integer literals, character literals are passed
as a strong from lexer to parser then interpreted there.  This is just as
awkward as it was for integer literals, without the excuse that we used to
need the information about the dts version to process them correctly.

So, move character literal processing back to the lexer as well, cleaning
things up.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 dtc-lexer.l  | 20 +++++++++++++++++---
 dtc-parser.y | 34 +---------------------------------
 2 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/dtc-lexer.l b/dtc-lexer.l
index ba5d150..0821bde 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -167,9 +167,23 @@ static void lexical_error(const char *fmt, ...);
 		}
 
 <*>{CHAR_LITERAL}	{
-			yytext[yyleng-1] = '\0';
-			yylval.literal = xstrdup(yytext+1);
-			DPRINT("Character literal: %s\n", yylval.literal);
+			struct data d;
+			DPRINT("Character literal: %s\n", yytext);
+
+			d = data_copy_escape_string(yytext+1, yyleng-2);
+			if (d.len == 1) {
+				lexical_error("Empty character literal");
+				yylval.integer = 0;
+				return DT_CHAR_LITERAL;
+			}
+
+			yylval.integer = (unsigned char)d.val[0];
+
+			if (d.len > 2)
+				lexical_error("Character literal has %d"
+					      " characters instead of 1",
+					      d.len - 1);
+
 			return DT_CHAR_LITERAL;
 		}
 
diff --git a/dtc-parser.y b/dtc-parser.y
index 0ce0815..efe81dd 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -32,13 +32,10 @@ extern void yyerror(char const *s);
 
 extern struct boot_info *the_boot_info;
 extern bool treesource_error;
-
-static unsigned char eval_char_literal(const char *s);
 %}
 
 %union {
 	char *propnodename;
-	char *literal;
 	char *labelref;
 	unsigned int cbase;
 	uint8_t byte;
@@ -65,7 +62,7 @@ static unsigned char eval_char_literal(const char *s);
 %token DT_DEL_NODE
 %token <propnodename> DT_PROPNODENAME
 %token <integer> DT_LITERAL
-%token <literal> DT_CHAR_LITERAL
+%token <integer> DT_CHAR_LITERAL
 %token <cbase> DT_BASE
 %token <byte> DT_BYTE
 %token <data> DT_STRING
@@ -336,9 +333,6 @@ arrayprefix:
 integer_prim:
 	  DT_LITERAL
 	| DT_CHAR_LITERAL
-		{
-			$$ = eval_char_literal($1);
-		}
 	| '(' integer_expr ')'
 		{
 			$$ = $2;
@@ -482,29 +476,3 @@ void print_error(char const *fmt, ...)
 void yyerror(char const *s) {
 	print_error("%s", s);
 }
-
-static unsigned char eval_char_literal(const char *s)
-{
-	int i = 1;
-	char c = s[0];
-
-	if (c == '\0')
-	{
-		print_error("empty character literal");
-		return 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;
-}
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/10] Die on failed /incbin/ seeks

[David Gibson]

Failing to open an input file, with /include/ or /incbin/ is treated as
immediately fatal inside srcfile_relative_open().  However, failint to
seek() to the requested offset in an /incbin/ is not.  This is a bit oddly
inconsistent, and leaves us with a strange case that's awkward to deal with
down the line.

So, get rid of it and have failed seeks on an /incbin/ be immediately
fatal.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 dtc-parser.y | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index efe81dd..bed857e 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -226,10 +226,9 @@ propdata:
 
 			if ($6 != 0)
 				if (fseek(f, $6, SEEK_SET) != 0)
-					print_error("Couldn't seek to offset %llu in \"%s\": %s",
-						     (unsigned long long)$6,
-						     $4.val,
-						     strerror(errno));
+					die("Couldn't seek to offset %llu in \"%s\": %s",
+					    (unsigned long long)$6, $4.val,
+					    strerror(errno));
 
 			d = data_copy_file(f, $8);
 
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/10] Correct locations in parser error messaes

[David Gibson]

The print_error() function used in several places in the parser uses the
location information in yylloc to describe the location of the error.
This is not correct in most cases.  yylloc gives the location of the
lookahead token, whereas the error is generally associated with one of
the already parsed non-terminals.

This patch corrects this, adding a location parameter to print_error() and
supplying it with the appropriate bison @N symbols.

This probably breaks yacc compatiblity, but too bad - accurate error
messages are more important.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 dtc-parser.y | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index bed857e..7ee436f 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -17,17 +17,14 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
  *                                                                   USA
  */
-
 %{
 #include <stdio.h>
 
 #include "dtc.h"
 #include "srcpos.h"
 
-YYLTYPE yylloc;
-
 extern int yylex(void);
-extern void print_error(char const *fmt, ...);
+extern void print_error(YYLTYPE *loc, char const *fmt, ...);
 extern void yyerror(char const *s);
 
 extern struct boot_info *the_boot_info;
@@ -148,7 +145,7 @@ devicetree:
 			if (target)
 				merge_nodes(target, $3);
 			else
-				print_error("label or path, '%s', not found", $2);
+				print_error(&@2, "label or path, '%s', not found", $2);
 			$$ = $1;
 		}
 	| devicetree DT_DEL_NODE DT_REF ';'
@@ -156,7 +153,7 @@ devicetree:
 			struct node *target = get_node_by_ref($1, $3);
 
 			if (!target)
-				print_error("label or path, '%s', not found", $3);
+				print_error(&@3, "label or path, '%s', not found", $3);
 			else
 				delete_node(target);
 
@@ -276,7 +273,7 @@ arrayprefix:
 			if ((bits !=  8) && (bits != 16) &&
 			    (bits != 32) && (bits != 64))
 			{
-				print_error("Only 8, 16, 32 and 64-bit elements"
+				print_error(&@2, "Only 8, 16, 32 and 64-bit elements"
 					    " are currently supported");
 				bits = 32;
 			}
@@ -302,7 +299,7 @@ arrayprefix:
 				 * mask), all bits are one.
 				 */
 				if (($2 > mask) && (($2 | mask) != -1ULL))
-					print_error(
+					print_error(&@2,
 						"integer value out of range "
 						"%016lx (%d bits)", $1.bits);
 			}
@@ -318,7 +315,7 @@ arrayprefix:
 							  REF_PHANDLE,
 							  $2);
 			else
-				print_error("References are only allowed in "
+				print_error(&@2, "References are only allowed in "
 					    "arrays with 32-bit elements.");
 
 			$$.data = data_append_integer($1.data, val, $1.bits);
@@ -438,7 +435,7 @@ subnodes:
 		}
 	| subnode propdef
 		{
-			print_error("syntax error: properties must precede subnodes");
+			print_error(&@2, "syntax error: properties must precede subnodes");
 			YYERROR;
 		}
 	;
@@ -461,17 +458,18 @@ subnode:
 
 %%
 
-void print_error(char const *fmt, ...)
+void print_error(YYLTYPE *loc, char const *fmt, ...)
 {
 	va_list va;
 
 	va_start(va, fmt);
-	srcpos_verror(&yylloc, "Error", fmt, va);
+	srcpos_verror(loc, "Error", fmt, va);
 	va_end(va);
 
 	treesource_error = true;
 }
 
-void yyerror(char const *s) {
-	print_error("%s", s);
+void yyerror(char const *s)
+{
+	print_error(&yylloc, "%s", s);
 }
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/10] Clean up parser error messages

[David Gibson]

Generally edit parser error messages for brevity and clarity.  Replace
the print_error() function with a a new macro for brevity and clarity in
the source.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 dtc-parser.y | 44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 7ee436f..42c4d75 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -24,8 +24,12 @@
 #include "srcpos.h"
 
 extern int yylex(void);
-extern void print_error(YYLTYPE *loc, char const *fmt, ...);
 extern void yyerror(char const *s);
+#define ERROR(loc, ...) \
+	do { \
+		srcpos_error((loc), "Error", __VA_ARGS__); \
+		treesource_error = true; \
+	} while (0)
 
 extern struct boot_info *the_boot_info;
 extern bool treesource_error;
@@ -145,17 +149,18 @@ devicetree:
 			if (target)
 				merge_nodes(target, $3);
 			else
-				print_error(&@2, "label or path, '%s', not found", $2);
+				ERROR(&@2, "Label or path %s not found", $2);
 			$$ = $1;
 		}
 	| devicetree DT_DEL_NODE DT_REF ';'
 		{
 			struct node *target = get_node_by_ref($1, $3);
 
-			if (!target)
-				print_error(&@3, "label or path, '%s', not found", $3);
-			else
+			if (target)
 				delete_node(target);
+			else
+				ERROR(&@3, "Label or path %s not found", $3);
+
 
 			$$ = $1;
 		}
@@ -271,10 +276,9 @@ arrayprefix:
 			bits = $2;
 
 			if ((bits !=  8) && (bits != 16) &&
-			    (bits != 32) && (bits != 64))
-			{
-				print_error(&@2, "Only 8, 16, 32 and 64-bit elements"
-					    " are currently supported");
+			    (bits != 32) && (bits != 64)) {
+				ERROR(&@2, "Array elements must be"
+				      " 8, 16, 32 or 64-bits");
 				bits = 32;
 			}
 
@@ -299,9 +303,8 @@ arrayprefix:
 				 * mask), all bits are one.
 				 */
 				if (($2 > mask) && (($2 | mask) != -1ULL))
-					print_error(&@2,
-						"integer value out of range "
-						"%016lx (%d bits)", $1.bits);
+					ERROR(&@2, "Value out of range for"
+					      " %d-bit array element", $1.bits);
 			}
 
 			$$.data = data_append_integer($1.data, $2, $1.bits);
@@ -315,7 +318,7 @@ arrayprefix:
 							  REF_PHANDLE,
 							  $2);
 			else
-				print_error(&@2, "References are only allowed in "
+				ERROR(&@2, "References are only allowed in "
 					    "arrays with 32-bit elements.");
 
 			$$.data = data_append_integer($1.data, val, $1.bits);
@@ -435,7 +438,7 @@ subnodes:
 		}
 	| subnode propdef
 		{
-			print_error(&@2, "syntax error: properties must precede subnodes");
+			ERROR(&@2, "Properties must precede subnodes");
 			YYERROR;
 		}
 	;
@@ -458,18 +461,7 @@ subnode:
 
 %%
 
-void print_error(YYLTYPE *loc, char const *fmt, ...)
-{
-	va_list va;
-
-	va_start(va, fmt);
-	srcpos_verror(loc, "Error", fmt, va);
-	va_end(va);
-
-	treesource_error = true;
-}
-
 void yyerror(char const *s)
 {
-	print_error(&yylloc, "%s", s);
+	ERROR(&yylloc, "%s", s);
 }
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [0/10] dtc: Assorted parsing and error reporting cleanups

[Jon Loeliger]

> This series consists of a number of small fixes and cleanups, mostly
> to error reporting in the lexer and parser.  I hit these as minor
> annoyances while experimenting with improved expression and schema
> support.
> 
> I'm sending these out for some review, before committing them to the
> main branch.
> 
> These patches are also in the 'errors' branch on the repository - note
> that I'm treating that as an experimental branch, and therefore
> subject to rebasing at whim.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

With one typo correction, s/strong/string/, in patch 07/10's
commit message, please:

Acked-by: Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>

for the whole series.

Thanks,
jdl
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [0/10] dtc: Assorted parsing and error reporting cleanups

[David Gibson]

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

On Fri, Jan 03, 2014 at 01:51:25PM -0600, Jon Loeliger wrote:
> > This series consists of a number of small fixes and cleanups, mostly
> > to error reporting in the lexer and parser.  I hit these as minor
> > annoyances while experimenting with improved expression and schema
> > support.
> > 
> > I'm sending these out for some review, before committing them to the
> > main branch.
> > 
> > These patches are also in the 'errors' branch on the repository - note
> > that I'm treating that as an experimental branch, and therefore
> > subject to rebasing at whim.
> > 
> 
> With one typo correction, s/strong/string/, in patch 07/10's
> commit message, please:
> 
> Acked-by: Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>

Thanks.  I fixed that and one or two similarly trivial errors and
committed the series.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]