Re: [PATCH] libfdt: fdt_path_offset_*(): Fix handling of paths with options in them

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

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

On Sat, May 30, 2015 at 09:52:23AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 29-05-15 14:26, David Gibson wrote:
> >On Thu, May 21, 2015 at 03:00:52PM +0200, Hans de Goede wrote:
> >>This is another approach at fixing the issues with paths with have options
> >>appended seperated by a ':' character.
> >>
> >>commit b4150b59ae ("libfdt: Add fdt_path_offset_namelen()")
> >>
> >>Is related to this, it allows the caller to specify to only look at part
> >>of the passed in path. But as experience with using this in the kernel has
> >>shown using this properly is quite hard since the options itself may have
> >>a '/' in them, also see the comment above the new fdt_path_next_seperator
> >>helper this commit adds.
> >>
> >>So this commit, which currently is being used by u-boot, instead simply
> >>teaches fdt_path_offset() to just do the right thing when it encounters
> >>paths with a ':' in them.
> >
> >I dislike this - it's building into the core path handling something
> >related to how external things happen to glue extra options on there.
> >
> >I also don't see why it's necessary.  ':' shouldn't appear in paths,
> >so why can't you just strchr() for the first ':', pass the first path
> >to path_offset_namelen, and the last part to your option parsing code?
> 
> When I wrote this (I was a bit slow in submitting it upstream)
> path_offset_namelen did not exist yet.
> 
> But more importantly the Linux kernel is also doing the handling of ':'
> at the same low level, see __of_find_node_by_path in drivers/of/base.c
> in the kernel, which is the kernel equivalent of fdt_path_offset.
> 
> So to me it seems best to also handle this at the same level in libfdt
> rather then expecting callers to deal with this, as that will lead to
> more and more callers needing to be fixed when the same construct gets
> used in more places.

No, sorry.  It might be used in several places, but it's still a long
way from being universal enough that I'm willing to fold it into the
core handling.

A helper function to handle this case, I'd be happy to apply.  Folding
it into the core semantics, no.

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