On Thu, Jul 16, 2015 at 01:10:20PM +0200, Thierry Reding wrote:
> From: Thierry Reding <tred...@nvidia.com>
> 
> Given a device tree node and a property name, the new fdt_find_string()
> 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 <tred...@nvidia.com>

This also looks good, except for the name.  I'd prefer
'fdt_stringlist_search()' again to match the existing
'fdt_stringlist_contains()'.

Speaking of which, fdt_stringlist_contains() could be reimplemented in
terms of this function.

> ---
> Changes in v2:
> - handle non-NUL-terminated properties more gracefully
> - improve documentation
> 
>  libfdt/fdt_ro.c | 30 ++++++++++++++++++++++++++++++
>  libfdt/libfdt.h | 22 ++++++++++++++++++++++
>  tests/strings.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 92 insertions(+)
> 
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index cf55c71df79c..39b84b1cea60 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -563,6 +563,36 @@ int fdt_count_strings(const void *fdt, int nodeoffset, 
> const char *property)
>       return count;
>  }
>  
> +int fdt_find_string(const void *fdt, int nodeoffset, const char *property,
> +                 const char *string)
> +{
> +     int length, len, index = 0;
> +     const char *list, *end;
> +
> +     list = fdt_getprop(fdt, nodeoffset, property, &length);
> +     if (!list)
> +             return -length;
> +
> +     len = strlen(string) + 1;
> +     end = list + length;
> +
> +     while (list < end) {
> +             length = strnlen(list, end - list) + 1;
> +
> +             /* Abort if the last string isn't properly NUL-terminated. */
> +             if (list + length > end)
> +                     return -FDT_ERR_BADVALUE;
> +
> +             if (length == len && memcmp(list, string, length) == 0)
> +                     return index;
> +
> +             list += length;
> +             index++;
> +     }
> +
> +     return -FDT_ERR_NOTFOUND;
> +}
> +
>  int fdt_node_check_compatible(const void *fdt, int nodeoffset,
>                             const char *compatible)
>  {
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index bf60c1593e4e..e64680dd741c 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -885,6 +885,28 @@ int fdt_stringlist_contains(const char *strlist, int 
> listlen, const char *str);
>   */
>  int fdt_count_strings(const void *fdt, int nodeoffset, const char *property);
>  
> +/**
> + * fdt_find_string - find a string in a string list and return its index
> + * @fdt: pointer to the device tree blob
> + * @nodeoffset: offset of a tree node
> + * @property: name of the property containing the string list
> + * @string: string to look up in the string list
> + *
> + * Note that it is possible for this function to succeed on property values
> + * that are not NUL-terminated. That's because the function will stop after
> + * finding the first occurrence of @string. This can for example happen with
> + * small-valued cell properties, such as #address-cells, when searching for
> + * the empty string.
> + *
> + * @return:
> + *   the index of the string in the list of strings
> + *   -FDT_ERR_BADVALUE if the property value is not NUL-terminated
> + *   -FDT_ERR_NOTFOUND if the property does not exist or does not contain
> + *                     the given string
> + */
> +int fdt_find_string(const void *fdt, int nodeoffset, const char *property,
> +                 const char *string);
> +
>  /**********************************************************************/
>  /* Read-only functions (addressing related)                           */
>  /**********************************************************************/
> diff --git a/tests/strings.c b/tests/strings.c
> index e10d9ece5a3e..29c49bfcc78c 100644
> --- a/tests/strings.c
> +++ b/tests/strings.c
> @@ -40,6 +40,24 @@ static void check_expected_failure(const void *fdt, const 
> char *path,
>       err = fdt_count_strings(fdt, offset, "#address-cells");
>       if (err != -FDT_ERR_BADVALUE)
>               FAIL("unexpectedly succeeded in parsing #address-cells\n");
> +
> +     err = fdt_find_string(fdt, offset, "#address-cells", "foo");
> +     if (err != -FDT_ERR_BADVALUE)
> +             FAIL("found string in #address-cells: %d\n", err);
> +
> +     /*
> +      * Note that the #address-cells property contains a small 32-bit
> +      * unsigned integer, hence some bytes will be zero, and searching for
> +      * the empty string will succeed.
> +      *
> +      * The reason for this oddity is that the function will exit when the
> +      * first occurrence of the string is found, but in order to determine
> +      * that the property does not contain a valid string list it would
> +      * need to process the whole value.
> +      */
> +     err = fdt_find_string(fdt, offset, "#address-cells", "");
> +     if (err != 0)
> +             FAIL("empty string not found in #address-cells: %d\n", err);
>  }
>  
>  static void check_string_count(const void *fdt, const char *path,
> @@ -61,6 +79,23 @@ static void check_string_count(const void *fdt, const char 
> *path,
>                    path, property, err, count);
>  }
>  
> +static void check_string_index(const void *fdt, const char *path,
> +                            const char *property, const char *string,
> +                            int index)
> +{
> +     int offset, err;
> +
> +     offset = fdt_path_offset(fdt, path);
> +     if (offset < 0)
> +             FAIL("Couldn't find path %s", path);
> +
> +     err = fdt_find_string(fdt, offset, property, string);
> +
> +     if (err != index)
> +             FAIL("Index of %s in property %s of node %s is %d, expected 
> %d\n",
> +                  string, property, path, err, index);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>       void *fdt;
> @@ -78,5 +113,10 @@ int main(int argc, char *argv[])
>       check_string_count(fdt, "/device", "compatible", 2);
>       check_string_count(fdt, "/device", "big-endian", 0);
>  
> +     check_string_index(fdt, "/", "compatible", "test-strings", 0);
> +     check_string_index(fdt, "/device", "compatible", "foo", 0);
> +     check_string_index(fdt, "/device", "compatible", "bar", 1);
> +     check_string_index(fdt, "/device", "big-endian", "baz", -1);
> +
>       PASS();
>  }

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

Reply via email to