[U-Boot,02/23] fdt: Add a function to get the index of a string

From: Thierry Reding <treding@nvidia.com>

Given a device tree node and a property name, the fdt_get_string_index()
function will look up a given string in the string list contained in the
property's value and return its index.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/libfdt.h    | 11 +++++++++++
 lib/libfdt/fdt_ro.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

Hi Thierry,

On 18 August 2014 01:16, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Given a device tree node and a property name, the fdt_get_string_index()
> function will look up a given string in the string list contained in the
> property's value and return its index.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/libfdt.h    | 11 +++++++++++
>  lib/libfdt/fdt_ro.c | 28 ++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
>
> diff --git a/include/libfdt.h b/include/libfdt.h
> index e7f991b388cf..4d7fb2681669 100644
> --- a/include/libfdt.h
> +++ b/include/libfdt.h
> @@ -857,6 +857,17 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
>   */
>  int fdt_stringlist_contains(const char *strlist, int listlen, const char *str);
>
> +/**
> + * fdt_get_string_index - get the index of a string in a string list
> + * @fdt: pointer to the device tree blob
> + * @node: offset of the node
> + * @property: name of the property containing the string list
> + * @string: string to look up in the string list
> + * @return the index of the string or negative on error
> + */
> +int fdt_get_string_index(const void *fdt, int node, const char *property,
> +                        const char *string);
> +
>  /**********************************************************************/
>  /* Read-only functions (addressing related)                           */
>  /**********************************************************************/
> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
> index 17cd11333c1e..f211c64ca7ca 100644
> --- a/lib/libfdt/fdt_ro.c
> +++ b/lib/libfdt/fdt_ro.c
> @@ -14,6 +14,8 @@
>
>  #include "libfdt_internal.h"
>
> +#define max(x, y) (((x) < (y)) ? (y) : (x))
> +
>  static int _fdt_nodename_eq(const void *fdt, int offset,
>                             const char *s, int len)
>  {
> @@ -491,6 +493,32 @@ int fdt_stringlist_contains(const char *strlist, int listlen, const char *str)
>         return 0;
>  }
>
> +int fdt_get_string_index(const void *fdt, int node, const char *property,
> +                        const char *string)
> +{
> +       const char *list, *end;
> +       int len, index = 0;
> +
> +       list = fdt_getprop(fdt, node, property, &len);
> +       if (!list)
> +               return len;
> +
> +       end = list + len;
> +
> +       while (list < end) {
> +               int n = strlen(string);

This can go outside the loop.

> +               int m = strlen(list);
> +
> +               if (n == m && memcmp(list, string, n) == 0)
> +                       return index;
> +
> +               list += max(n, m) + 1;

I worry that if n > m this will end up in the middle of a the next
string in the list. What is the intention here?

> +               index++;
> +       }
> +
> +       return -FDT_ERR_NOTFOUND;
> +}
> +
>  int fdt_node_check_compatible(const void *fdt, int nodeoffset,
>                               const char *compatible)
>  {
> --
> 2.0.4
>

Regards,
Simon

On Mon, Aug 18, 2014 at 11:58:09AM -0600, Simon Glass wrote:
> On 18 August 2014 01:16, Thierry Reding <thierry.reding@gmail.com> wrote:
[...]
> > +int fdt_get_string_index(const void *fdt, int node, const char *property,
> > +                        const char *string)
> > +{
> > +       const char *list, *end;
> > +       int len, index = 0;
> > +
> > +       list = fdt_getprop(fdt, node, property, &len);
> > +       if (!list)
> > +               return len;
> > +
> > +       end = list + len;
> > +
> > +       while (list < end) {
> > +               int n = strlen(string);
> 
> This can go outside the loop.

Indeed.

> > +               int m = strlen(list);
> > +
> > +               if (n == m && memcmp(list, string, n) == 0)
> > +                       return index;
> > +
> > +               list += max(n, m) + 1;
> 
> I worry that if n > m this will end up in the middle of a the next
> string in the list. What is the intention here?

I can no longer recall why that's there. It certainly seems wrong since
if the current substring isn't what we're looking for the function
should proceed to the next one, which is always m + 1 characters away.

I'll replace this with list += m + 1.

Thierry