dtc: Simplify error handling for unparseable input

dtc: Simplify error handling for unparseable input

[David Gibson]

Currently, main() tests if it got a valid input tree from whichever
dt_from_*() function it invoked and if not, die()s.  For one thing,
this test has, for no good reason, three different ways for those
functions to communicate a failure to provide input (bi NULL, bi->dt
NULL, or bi->error non-zero).  For another, in every case save one, if
the dt_from_*() functions are unable to provide input they will
immediately die() (with a more specific error message) rather than
proceeding to the test in main().

Therefore, this patch removes this test, making the one case that
could have triggered it (in dt_from_source()) call die() directly
instead.  With this change, the error field in struct boot_info is now
unused, so remove it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

---
 dtc.c        |    3 ---
 dtc.h        |    1 -
 livetree.c   |    1 -
 treesource.c |    6 ++++--
 4 files changed, 4 insertions(+), 7 deletions(-)

Index: dtc/dtc.h
===================================================================
--- dtc.orig/dtc.h	2008-03-24 14:33:33.000000000 +1100
+++ dtc/dtc.h	2008-03-24 14:33:34.000000000 +1100
@@ -232,7 +232,6 @@
 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,
Index: dtc/livetree.c
===================================================================
--- dtc.orig/livetree.c	2008-03-24 14:18:44.000000000 +1100
+++ dtc/livetree.c	2008-03-24 14:33:34.000000000 +1100
@@ -172,7 +172,6 @@
 	bi = xmalloc(sizeof(*bi));
 	bi->reservelist = reservelist;
 	bi->dt = tree;
-	bi->error = 0;
 
 	return bi;
 }
Index: dtc/dtc.c
===================================================================
--- dtc.orig/dtc.c	2008-03-24 14:35:05.000000000 +1100
+++ dtc/dtc.c	2008-03-24 14:35:10.000000000 +1100
@@ -201,9 +201,6 @@
 	if (inf && inf->file != stdin)
 		fclose(inf->file);
 
-	if (! bi || ! bi->dt || bi->error)
-		die("Couldn't read input tree\n");
-
 	fill_fullpaths(bi->dt, "");
 	process_checks(force, bi);
 
Index: dtc/treesource.c
===================================================================
--- dtc.orig/treesource.c	2008-03-24 14:33:44.000000000 +1100
+++ dtc/treesource.c	2008-03-24 14:35:52.000000000 +1100
@@ -36,9 +36,11 @@
 	yyin = srcpos_file->file;
 
 	if (yyparse() != 0)
-		return NULL;
+		die("Unable to parse input tree\n");
+
+	if (treesource_error)
+		die("Syntax error parsing input tree\n");
 
-	the_boot_info->error = treesource_error;
 	return the_boot_info;
 }
 

-- 
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: dtc: Simplify error handling for unparseable input

[Scott Wood]

On Mon, Mar 24, 2008 at 02:44:24PM +1100, David Gibson wrote:
> Index: dtc/dtc.h
> ===================================================================
> --- dtc.orig/dtc.h	2008-03-24 14:33:33.000000000 +1100
> +++ dtc/dtc.h	2008-03-24 14:33:34.000000000 +1100
> @@ -232,7 +232,6 @@
>  struct boot_info {
>  	struct reserve_info *reservelist;
>  	struct node *dt;		/* the device tree */
> -	int error;
>  };

If you remove this, there'll be no way to indicate semantic errors other
than die() (the NULL approaches are no good, since they inhibit recovery),
which is suboptimal if the error is not immediately fatal.

-Scott

Re: dtc: Simplify error handling for unparseable input

[David Gibson]

On Mon, Mar 24, 2008 at 12:36:41PM -0500, Scott Wood wrote:
> On Mon, Mar 24, 2008 at 02:44:24PM +1100, David Gibson wrote:
> > Index: dtc/dtc.h
> > ===================================================================
> > --- dtc.orig/dtc.h	2008-03-24 14:33:33.000000000 +1100
> > +++ dtc/dtc.h	2008-03-24 14:33:34.000000000 +1100
> > @@ -232,7 +232,6 @@
> >  struct boot_info {
> >  	struct reserve_info *reservelist;
> >  	struct node *dt;		/* the device tree */
> > -	int error;
> >  };
> 
> If you remove this, there'll be no way to indicate semantic errors other
> than die() (the NULL approaches are no good, since they inhibit recovery),
> which is suboptimal if the error is not immediately fatal.

But everything is immediately fatal.  When we have a *real* example of
something that's not, we can restore an error code.

-- 
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: dtc: Simplify error handling for unparseable input

[Scott Wood]

On Tue, Mar 25, 2008 at 12:28:05PM +1100, David Gibson wrote:
> On Mon, Mar 24, 2008 at 12:36:41PM -0500, Scott Wood wrote:
> > If you remove this, there'll be no way to indicate semantic errors other
> > than die() (the NULL approaches are no good, since they inhibit recovery),
> > which is suboptimal if the error is not immediately fatal.
> 
> But everything is immediately fatal.  When we have a *real* example of
> something that's not, we can restore an error code.

Failed binary includes are not immediately fatal.

-Scott

Re: dtc: Simplify error handling for unparseable input

[David Gibson]

On Tue, Mar 25, 2008 at 09:36:19AM -0500, Scott Wood wrote:
> On Tue, Mar 25, 2008 at 12:28:05PM +1100, David Gibson wrote:
> > On Mon, Mar 24, 2008 at 12:36:41PM -0500, Scott Wood wrote:
> > > If you remove this, there'll be no way to indicate semantic errors other
> > > than die() (the NULL approaches are no good, since they inhibit recovery),
> > > which is suboptimal if the error is not immediately fatal.
> > 
> > But everything is immediately fatal.  When we have a *real* example of
> > something that's not, we can restore an error code.
> 
> Failed binary includes are not immediately fatal.

And is there any advantage to having them not immediately fatal?

-- 
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: dtc: Simplify error handling for unparseable input

[Scott Wood]

David Gibson wrote:
> On Tue, Mar 25, 2008 at 09:36:19AM -0500, Scott Wood wrote:
>> On Tue, Mar 25, 2008 at 12:28:05PM +1100, David Gibson wrote:
>>> On Mon, Mar 24, 2008 at 12:36:41PM -0500, Scott Wood wrote:
>>>> If you remove this, there'll be no way to indicate semantic errors other
>>>> than die() (the NULL approaches are no good, since they inhibit recovery),
>>>> which is suboptimal if the error is not immediately fatal.
>>> But everything is immediately fatal.  When we have a *real* example of
>>> something that's not, we can restore an error code.
>> Failed binary includes are not immediately fatal.
> 
> And is there any advantage to having them not immediately fatal?

It's generally nice to the user if you can report as many bugs as you 
can rather than fail on the first one.

It's also nice to someone down the road trying to turn this code into a 
library if it passes return status up the call chain gracefully.

-Scott

Re: dtc: Simplify error handling for unparseable input

[David Gibson]

On Tue, Mar 25, 2008 at 05:10:07PM -0500, Scott Wood wrote:
> David Gibson wrote:
>> On Tue, Mar 25, 2008 at 09:36:19AM -0500, Scott Wood wrote:
>>> On Tue, Mar 25, 2008 at 12:28:05PM +1100, David Gibson wrote:
>>>> On Mon, Mar 24, 2008 at 12:36:41PM -0500, Scott Wood wrote:
>>>>> If you remove this, there'll be no way to indicate semantic errors 
>>>>> other
>>>>> than die() (the NULL approaches are no good, since they inhibit 
>>>>> recovery),
>>>>> which is suboptimal if the error is not immediately fatal.
>>>> But everything is immediately fatal.  When we have a *real* example of
>>>> something that's not, we can restore an error code.
>>> Failed binary includes are not immediately fatal.
>> And is there any advantage to having them not immediately fatal?
>
> It's generally nice to the user if you can report as many bugs as you can 
> rather than fail on the first one.

Hrm, I guess.  There's only so far it's worth going to achieve that
though.

> It's also nice to someone down the road trying to turn this code into a 
> library if it passes return status up the call chain gracefully.

Can you think of any reason we'd want to do that?  And that would
require fixing so many other places that the two cases which do return
an error (that's including binary includes) hardly signify.

-- 
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: dtc: Simplify error handling for unparseable input

[David Gibson]

On Tue, Mar 25, 2008 at 05:10:07PM -0500, Scott Wood wrote:
> David Gibson wrote:
>> On Tue, Mar 25, 2008 at 09:36:19AM -0500, Scott Wood wrote:
>>> On Tue, Mar 25, 2008 at 12:28:05PM +1100, David Gibson wrote:
>>>> On Mon, Mar 24, 2008 at 12:36:41PM -0500, Scott Wood wrote:
>>>>> If you remove this, there'll be no way to indicate semantic errors 
>>>>> other
>>>>> than die() (the NULL approaches are no good, since they inhibit 
>>>>> recovery),
>>>>> which is suboptimal if the error is not immediately fatal.
>>>> But everything is immediately fatal.  When we have a *real* example of
>>>> something that's not, we can restore an error code.
>>> Failed binary includes are not immediately fatal.
>> And is there any advantage to having them not immediately fatal?
>
> It's generally nice to the user if you can report as many bugs as you can 
> rather than fail on the first one.

Oh.. and this patch doesn't actually preclude that.  We still have the
treesource_error variable and can report errors that way during the
parse.  The die() just comes at the end of dt_from_source(), instead
of in main().

-- 
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: dtc: Simplify error handling for unparseable input

[Scott Wood]

On Wed, Mar 26, 2008 at 10:52:02AM +1100, David Gibson wrote:
> On Tue, Mar 25, 2008 at 05:10:07PM -0500, Scott Wood wrote:
> > It's generally nice to the user if you can report as many bugs as you can 
> > rather than fail on the first one.
> 
> Hrm, I guess.  There's only so far it's worth going to achieve that
> though.
> 
> > It's also nice to someone down the road trying to turn this code into a 
> > library if it passes return status up the call chain gracefully.
> 
> Can you think of any reason we'd want to do that? 

Anything that wants to do something automated with device tree source
that doesn't fit neatly into command-line execution.  Nothing specific.

> And that would require fixing so many other places that the two cases
> which do return an error (that's including binary includes) hardly
> signify.

Yeah, I just wanted to avoid going further down that path if possible.

I won't protest too loudly if the general consensus is to just die(),
though.

-Scott