[PATCH] of: Make of_find_node_by_path() traverse /aliases for relative paths.

[Grant Likely]

On Tue, Mar 1, 2011 at 4:15 PM, David Daney <ddaney at caviumnetworks.com> wrote:
> Currently all paths passed to of_find_node_by_path() must begin with a
> '/', indicating a full path to the desired node.
>
> Augment the look-up code so that if a path does *not* begin with '/',
> the path is used as the name of an /aliases property.  The value of
> this alias is then used as the full node path to be found.

Nice, thanks David!

However, there is one more use case that this patch needs to handle.
Right now it will support:

"/full/path/to/node"
and
"alias"

but in OF terms it is also valid to specify:

"alias/relative/path/to/child"

How I would handle this is to split on the first '/', use the front
half to look up the path to the alias, and then concatenate the alias
path with the child node relative path before doing the full lookup.
Although that might not be the best way since it requires doing a copy
to a temporary buffer.

Alternately, it could be implemented with a multi test loop that does
something like:

for (; np; np = np->allnext) {
        int len = strlen(alias_path);
        if (np->full_name &&
            (strncmp(np->full_name, alias_path, len) == 0) &&
            (np->full_name[len] == '/') &&
            (strncmp(&np->full_name[len+1], relative_path,
strlen(relative_path)) == 0)) {
                of_node_get(np);
                goto out;
        }
}


[...]
> +
> +       if (path[0] != '/') {
> +               aliases = of_find_node_by_path("/aliases");
> +               if (!aliases)
> +                       goto out;
> +               path = of_get_property(aliases, path, NULL);
> +               /*
> +                * The alias must begin with '/', otherwise we could
> +                * get stuck in an endless loop and blow out the
> +                * stack.
> +                */

I don't think this is true (the endless loop part).  I don't see how
this would enter into a recursive set of calls other than the call
of_find_node_by_path("/aliases") which is hard coded with a '/' in the
first character.  The test for path[0] != '/' looks unnecessary, or am
I missing something?

> +               if (!path || path[0] != '/')
> +                       goto out;
> +       }
> +
>        for (; np; np = np->allnext) {
>                if (np->full_name && (of_node_cmp(np->full_name, path) == 0)
>                    && of_node_get(np))
>                        break;
>        }
> +out:
> +       if (aliases)
> +               of_node_put(aliases);

Since there is only one error path that needs to do this
of_node_put(), I'd put it right into the error path and save one test.

g.