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:
pgp7SITUNzEj0.pgp
Description: PGP signature