[PATCH 3/3] Return a non-zero exit code if an error occurs during dts parsing.

[PATCH 3/3] Return a non-zero exit code if an error occurs during dts parsing.

[Scott Wood]

Previously, only failure to parse caused the reading of the tree to fail;
semantic errors that called yyerror() but not YYERROR only emitted a message,
without signalling make to stop the build.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 dtc-parser.y |    2 ++
 dtc.c        |    2 +-
 dtc.h        |    1 +
 livetree.c   |    1 +
 treesource.c |    3 +++
 5 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 8ed58e8..da7f6f5 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -28,6 +28,7 @@ int yylex(void);
 unsigned long long eval_literal(const char *s, int base, int bits);
 
 extern struct boot_info *the_boot_info;
+extern int treesource_error;
 
 %}
 
@@ -320,6 +321,7 @@ void yyerrorf(char const *s, ...)
 	vfprintf(stderr, s, va);
 	fprintf(stderr, "\n");
 
+	treesource_error = 1;
 	va_end(va);
 }
 
diff --git a/dtc.c b/dtc.c
index fb716f3..c1814c1 100644
--- a/dtc.c
+++ b/dtc.c
@@ -205,7 +205,7 @@ int main(int argc, char *argv[])
 	if (inf && inf->file != stdin)
 		fclose(inf->file);
 
-	if (! bi || ! bi->dt)
+	if (! bi || ! bi->dt || bi->error)
 		die("Couldn't read input tree\n");
 
 	process_checks(force, bi);
diff --git a/dtc.h b/dtc.h
index 9b89689..cba9d28 100644
--- a/dtc.h
+++ b/dtc.h
@@ -233,6 +233,7 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
 struct boot_info {
 	struct reserve_info *reservelist;
 	struct node *dt;		/* the device tree */
+	int error;
 };
 
 struct boot_info *build_boot_info(struct reserve_info *reservelist,
diff --git a/livetree.c b/livetree.c
index 6ba0846..7610e78 100644
--- a/livetree.c
+++ b/livetree.c
@@ -172,6 +172,7 @@ struct boot_info *build_boot_info(struct reserve_info *reservelist,
 	bi = xmalloc(sizeof(*bi));
 	bi->reservelist = reservelist;
 	bi->dt = tree;
+	bi->error = 0;
 
 	return bi;
 }
diff --git a/treesource.c b/treesource.c
index e7d580f..980bda7 100644
--- a/treesource.c
+++ b/treesource.c
@@ -25,10 +25,12 @@ extern FILE *yyin;
 extern int yyparse(void);
 
 struct boot_info *the_boot_info;
+int treesource_error;
 
 struct boot_info *dt_from_source(const char *fname)
 {
 	the_boot_info = NULL;
+	treesource_error = 0;
 
 	push_input_file(fname);
 
@@ -37,6 +39,7 @@ struct boot_info *dt_from_source(const char *fname)
 
 	fill_fullpaths(the_boot_info->dt, "");
 
+	the_boot_info->error = treesource_error;
 	return the_boot_info;
 }
 
-- 
1.5.3

Re: [PATCH 3/3] Return a non-zero exit code if an error occurs during dts parsing.

[David Gibson]

On Thu, Jan 03, 2008 at 05:43:33PM -0600, Scott Wood wrote:
> Previously, only failure to parse caused the reading of the tree to fail;
> semantic errors that called yyerror() but not YYERROR only emitted a message,
> without signalling make to stop the build.

This one, however, I don't like.

[snip]
> diff --git a/dtc.h b/dtc.h
> index 9b89689..cba9d28 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -233,6 +233,7 @@ struct reserve_info *add_reserve_entry(struct reserve_info *list,
>  struct boot_info {
>  	struct reserve_info *reservelist;
>  	struct node *dt;		/* the device tree */
> +	int error;
>  };

This is unequivocally wrong.  boot_info should have information about
the contents of the blob, not state information like the error.

If you're going to use an ugly global, then use it everywhere.

-- 
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 3/3] Return a non-zero exit code if an error occurs during dts parsing.

[Scott Wood]

On Fri, Jan 04, 2008 at 03:30:33PM +1100, David Gibson wrote:
> This is unequivocally wrong.  boot_info should have information about
> the contents of the blob, not state information like the error.

"This blob is invalid" *is* information about the contents of the blob.

> If you're going to use an ugly global, then use it everywhere.

Why go out of our way to make the code even less library-able/thread-safe?

-Scott

Re: [PATCH 3/3] Return a non-zero exit code if an error occurs during dts parsing.

[David Gibson]

On Sun, Jan 06, 2008 at 04:55:09PM -0600, Scott Wood wrote:
> On Fri, Jan 04, 2008 at 03:30:33PM +1100, David Gibson wrote:
> > This is unequivocally wrong.  boot_info should have information about
> > the contents of the blob, not state information like the error.
> 
> "This blob is invalid" *is* information about the contents of the blob.
> 
> > If you're going to use an ugly global, then use it everywhere.
> 
> Why go out of our way to make the code even less library-able/thread-safe?

It doesn't make it any less thread-safe.  A global variable used some
places is just as bad as a global variable used everywhere from that
point of view, and is more complicated.

-- 
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 3/3] Return a non-zero exit code if an error occurs during dts parsing.

[Scott Wood]

David Gibson wrote:
> On Sun, Jan 06, 2008 at 04:55:09PM -0600, Scott Wood wrote:
>> On Fri, Jan 04, 2008 at 03:30:33PM +1100, David Gibson wrote:
>>> This is unequivocally wrong.  boot_info should have information about
>>> the contents of the blob, not state information like the error.
>> "This blob is invalid" *is* information about the contents of the blob.
>>
>>> If you're going to use an ugly global, then use it everywhere.
>> Why go out of our way to make the code even less library-able/thread-safe?
> 
> It doesn't make it any less thread-safe.  A global variable used some
> places is just as bad as a global variable used everywhere from that
> point of view, and is more complicated.

But the knowledge of the fact that the boot_info struct is a global is 
isolated to the treesource code.  I don't see any reason to add another 
global at the *interface* level, much less that not doing so is 
"unequivocally wrong".

-Scott