Re: [PATCH v5 5/8] libfdt: Add fdt_setprop_inplace_namelen_partial

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

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

On Mon, Aug 22, 2016 at 12:07:21PM +0200, Maxime Ripard wrote:
> Hi David,
> 
> On Mon, Aug 01, 2016 at 01:36:42PM +1000, David Gibson wrote:
> > On Fri, Jul 29, 2016 at 11:55:48AM +0200, Maxime Ripard wrote:
> > > Add a function to modify inplace only a portion of a property..
> > > 
> > > This is especially useful when the property is an array of values, and you
> > > want to update one of them without changing the DT size.
> > > 
> > > Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > > Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > 
> > Applied, though I made a couple of tiny tweaks, see below.
> 
> Awesome, thanks!
> 
> Do you still have comments on the last 3 patches?

Sorry, I got sidetracked by other things.

The only remaining thing I'd like some changes in is the error
reporting.

The biggset problem is that in some places you are returning
BADSTRUCTURE for badly formatter fixup or other overlay properties.
BADSTRUCTURE should only be used when there is a problem in the
encoding of the tree structure itself, not a problem in the encoding
within a specific property or properties.

I don't think your code should ever generate BADSTRUCTURE directly
(but it could return it when it gets that value returned from a lower
level function).

What I was hoping to have a closer look at was to see if I'd prefer
just a single 'BADOVERLAY' error for any problem in te overlay
encoding, or if I'd prefer sveral: BADFIXUP, BADTARGET, or something
like that.  I'll try to get a closer look, but it probably won't
happen this week (I'm at KVM Forum).

-- 
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]