Re: dtc: Return code of fdt_get_path()

[David Gibson <dwg-8fk3Idey6ehBDgjK7y7TUQ@public.gmane.org>]

On Thu, Aug 07, 2008 at 04:08:20PM +1000, David Gibson wrote:
> On Wed, Aug 06, 2008 at 03:33:10PM +1000, David Gibson wrote:
> > On Tue, Aug 05, 2008 at 04:12:22PM -0500, Scott Wood wrote:
> > > The documentation for fdt_get_path() says it returns zero on success,  
> > > but it actually returns the length of the string.  Is there any  
> > > preference on which one to fix?
> > 
> > The documentation I think.  There may be existing users, and the
> > length of the path is potentially useful.
> > 
> > Although.. I think the confusion came because at one stage I planned
> > to have it return the full length of the path, even if the buffer was
> > smaller.  That would let the caller allocate a buffer of exactly the
> > right size on a second call.  Which is potentially useful, but not
> > compatible with returning NOSPACE as we do now.
> > 
> > Plus.. looking over that code again, I think there may be more
> > problems there - I think there may be cases where it runs out of space
> > but returns BADOFFSET or some error code instead of NOSPACE.
> 
> Ugh, yes, confirmed that it is buggy.  Not sure of the best way to fix
> this, I can see three ways to go here.
> 
> 	1. Minimally fix the error code bug, make sure NOSPACE is
> returned when the buffer is too small, length of path otherwise,
> update documentation to match.
> 	2. Make the code match the documentation: return 0 on success,
> NOSPACE if the buffer is too small.
> 	3. Update both code and documentation to match the semantics I
> think I must have intended at some point: always return the path
> length, even if it exceeds the buffer length; caller is responsible
> for checking if the path in the buffer is truncated.
> 
> (3) has the possibly useful property that you can use the call with a
> small buffer and if it fails the caller now knows how big a buffer it
> must allocate.  With (1) and (2) the caller's only way to deal with
> NOSPACE is to keep allocating bigger buffers until the call succeeds.
> However it makes it more complex for the caller to check the return
> value - not just that it's non-negative put also that it's less than
> the buffer size.  Plus its arguably stylistically inconsistent with
> the read-write functions which return NOSPACE when the buffer is
> exceeded with no indication of how much space is needed.

Ah.. except that on more detailed consideration I don't think (3) is
possible, without either maintaining a stack (which then has
allocation issues) or multiple passes through the tree.  And if we
can't return the path length in general, I don't see that there's a
lot of point returning it ever, so I'll make a patch implementing
option (2).

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