Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()
Hmm.. my reading of 1275 says that an alias pointing to another alias is not permitted, but I'm not terribly confident I'm not misreading it. Segher, do you know whether this is allowed? My reading is the same: if after expanding an alias the path does not start with "/", the search starts at the current package, otherwise it starts at the root node; and no further alias expansion is done. It's a corner case for sure, but at least the standard doesn't require us to implement recursive alias expansion; let's see if common practice does ;-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()
David Gibson wrote: On Thu, Aug 14, 2008 at 12:43:48PM -0500, Scott Wood wrote: On Thu, Aug 14, 2008 at 08:28:19AM -0500, Kumar Gala wrote: - if (*path != '/') - return -FDT_ERR_BADPATH; + /* see if we have an alias */ + if (*path != '/') { + const char *q; + int aliasoffset = fdt_path_offset(fdt, "/aliases"); + + if (aliasoffset < 0) + return -FDT_ERR_BADPATH; + + q = strchr(path, '/'); + if (!q) + q = end; + + p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL); + if (!p) + return -FDT_ERR_BADPATH; + offset = fdt_path_offset(fdt, p); + + p = q; + } Can we limit the recursion depth to avoid falling off the stack if an alias points to itself? Or if aliases pointing to aliases are disallowed, check for a leading '/' before recursively calling fdt_path_offset. Hmm.. my reading of 1275 says that an alias pointing to another alias is not permitted, but I'm not terribly confident I'm not misreading it. Segher, do you know whether this is allowed? The 1275 spec doesn't require multiple levels of aliasing, but my current implementation allows it and uses it for things like components of the network stack. If that's the case then, yes, we should not recurse if the alias value doesn't start with a /. In fact, if I factor out a fdt_get_alias() function, it should probably check the alias and return an error if it's not an absolute path. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()
On Thu, Aug 14, 2008 at 12:43:48PM -0500, Scott Wood wrote: > On Thu, Aug 14, 2008 at 08:28:19AM -0500, Kumar Gala wrote: > > - if (*path != '/') > > - return -FDT_ERR_BADPATH; > > + /* see if we have an alias */ > > + if (*path != '/') { > > + const char *q; > > + int aliasoffset = fdt_path_offset(fdt, "/aliases"); > > + > > + if (aliasoffset < 0) > > + return -FDT_ERR_BADPATH; > > + > > + q = strchr(path, '/'); > > + if (!q) > > + q = end; > > + > > + p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL); > > + if (!p) > > + return -FDT_ERR_BADPATH; > > + offset = fdt_path_offset(fdt, p); > > + > > + p = q; > > + } > > Can we limit the recursion depth to avoid falling off the stack if an > alias points to itself? Or if aliases pointing to aliases are > disallowed, check for a leading '/' before recursively calling > fdt_path_offset. Hmm.. my reading of 1275 says that an alias pointing to another alias is not permitted, but I'm not terribly confident I'm not misreading it. Segher, do you know whether this is allowed? If that's the case then, yes, we should not recurse if the alias value doesn't start with a /. In fact, if I factor out a fdt_get_alias() function, it should probably check the alias and return an error if it's not an absolute path. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()
On Thu, Aug 14, 2008 at 08:28:19AM -0500, Kumar Gala wrote: > - if (*path != '/') > - return -FDT_ERR_BADPATH; > + /* see if we have an alias */ > + if (*path != '/') { > + const char *q; > + int aliasoffset = fdt_path_offset(fdt, "/aliases"); > + > + if (aliasoffset < 0) > + return -FDT_ERR_BADPATH; > + > + q = strchr(path, '/'); > + if (!q) > + q = end; > + > + p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL); > + if (!p) > + return -FDT_ERR_BADPATH; > + offset = fdt_path_offset(fdt, p); > + > + p = q; > + } Can we limit the recursion depth to avoid falling off the stack if an alias points to itself? Or if aliases pointing to aliases are disallowed, check for a leading '/' before recursively calling fdt_path_offset. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()
> If the path doesn't start with '/' check to see if it matches some alias > under "/aliases" and substitute the matching alias value in the path > and retry the lookup. > > Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> Applied. jdl ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()
On Thu, Aug 14, 2008 at 08:28:19AM -0500, Kumar Gala wrote: > If the path doesn't start with '/' check to see if it matches some alias > under "/aliases" and substitute the matching alias value in the path > and retry the lookup. > > Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> Acked-by: David Gibson <[EMAIL PROTECTED]> -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev