[PATCH] fdtdec: use full path in fdtdec_get_alias_seq comparison

[Vladimir Oltean]

Hi Simon,

On Sat, 28 Mar 2020 at 22:06, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Vladimir,
>
> On Fri, 27 Mar 2020 at 11:29, Vladimir Oltean <olteanv at gmail.com> wrote:
> >
> > From: Vladimir Oltean <vladimir.oltean at nxp.com>
> >
> > Currently fdtdec_get_alias_seq() calls fdt_get_name() which returns only
> > the name of the leaf node. So it needs to also trim the path of the
> > alias to the leaf name only, leading to imperfect matches.
> >
> > This means that the following aliases:
> >
> >     /aliases {
> >         eth0 = "/dspi at 2120000/ethernet-switch at 0/ports/port at 0";
> >         eth1 = "/pcie at 1f0000000/pci at 0,5/ports/port at 0";
> >     };
> >
> > will make fdtdec_get_alias_seq to return a seq of zero for both aliases,
> > as the match will only take place on the "port at 0" portion.
> >
> > Fix this by calling fdt_get_path and comparing the full path of the
> > alias.
> >
> > Fixes: 5c33c9fdbb3f ("fdt: Add a function to get the alias sequence of a node")
> > Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com>
> > ---
> >  lib/fdtdec.c | 15 +++++++--------
> >  1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> > index eb11fc898e30..55c1b36e823b 100644
> > --- a/lib/fdtdec.c
> > +++ b/lib/fdtdec.c
> > @@ -455,13 +455,14 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
> >                          int *seqp)
> >  {
> >         int base_len = strlen(base);
> > -       const char *find_name;
> > -       int find_namelen;
> >         int prop_offset;
> > +       char path[64];
> > +       int path_len;
> >         int aliases;
> >
> > -       find_name = fdt_get_name(blob, offset, &find_namelen);
> > -       debug("Looking for '%s' at %d, name %s\n", base, offset, find_name);
> > +       fdt_get_path(blob, offset, path, sizeof(path));
>
> This is really slow. I would prefer not to change this for what seems
> like an odd case.
>
> Can you enable CONFIG_OF_LIVE instead?
>

No, why would I want to do that?
Why is this an odd case? I thought aliases are supposed to match on
full path, no?

> > +       path_len = strlen(path);
> > +       debug("Looking for '%s' at %d, path %s\n", base, offset, path);
> >
> >         aliases = fdt_path_offset(blob, "/aliases");
> >         for (prop_offset = fdt_first_property_offset(blob, aliases);
> > @@ -469,17 +470,15 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
> >              prop_offset = fdt_next_property_offset(blob, prop_offset)) {
> >                 const char *prop;
> >                 const char *name;
> > -               const char *slash;
> >                 int len, val;
> >
> >                 prop = fdt_getprop_by_offset(blob, prop_offset, &name, &len);
> >                 debug("   - %s, %s\n", name, prop);
> > -               if (len < find_namelen || *prop != '/' || prop[len - 1] ||
> > +               if (len < path_len || *prop != '/' || prop[len - 1] ||
> >                     strncmp(name, base, base_len))
> >                         continue;
> >
> > -               slash = strrchr(prop, '/');
> > -               if (strcmp(slash + 1, find_name))
> > +               if (strcmp(prop, path))
> >                         continue;
> >                 val = trailing_strtol(name);
> >                 if (val != -1) {
> > --
> > 2.17.1
> >
>
> Regards,
> Simon

Regards,
-Vladimir