diff mbox

[U-Boot,v4,08/13] libfdt: Add fdt_path_offset_namelen

Message ID 20160705082646.25044-9-maxime.ripard@free-electrons.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Maxime Ripard July 5, 2016, 8:26 a.m. UTC
Add a namelen variant of fdt_path_offset to retrieve the node offset using
only a fixed number of characters.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 include/libfdt.h    | 16 +++++++++++++++-
 lib/libfdt/fdt_ro.c | 18 ++++++++++--------
 2 files changed, 25 insertions(+), 9 deletions(-)

Comments

David Gibson July 6, 2016, 1:21 a.m. UTC | #1
On Tue, Jul 05, 2016 at 10:26:41AM +0200, Maxime Ripard wrote:
> Add a namelen variant of fdt_path_offset to retrieve the node offset using
> only a fixed number of characters.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  include/libfdt.h    | 16 +++++++++++++++-
>  lib/libfdt/fdt_ro.c | 18 ++++++++++--------
>  2 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/include/libfdt.h b/include/libfdt.h
> index 4643be5adf39..f13b01f08f71 100644
> --- a/include/libfdt.h
> +++ b/include/libfdt.h
> @@ -365,6 +365,17 @@ int fdt_subnode_offset_namelen(const void *fdt, int parentoffset,
>   */
>  int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name);
>  
> +/**
> + * fdt_path_offset_namelen - find a tree node based on substring
> + * @fdt: pointer to the device tree blob
> + * @path: full path of the node to locate
> + * @namelen: number of characters of name to consider
> + *
> + * Identical to fdt_path_offset(), but only examine the first
> + * namelen characters of path for matching the node path.
> + */
> +int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen);
> +
>  /**
>   * fdt_path_offset - find a tree node by its full path
>   * @fdt: pointer to the device tree blob
> @@ -387,7 +398,10 @@ int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name);
>   *	-FDT_ERR_BADSTRUCTURE,
>   *	-FDT_ERR_TRUNCATED, standard meanings.
>   */
> -int fdt_path_offset(const void *fdt, const char *path);
> +static inline int fdt_path_offset(const void *fdt, const char *path)
> +{
> +	return fdt_path_offset_namelen(fdt, path, strlen(path));
> +}
>  
>  /**
>   * fdt_get_name - retrieve the name of a given node
> diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
> index 6b737b211d2e..9cc98db6e2bf 100644
> --- a/lib/libfdt/fdt_ro.c
> +++ b/lib/libfdt/fdt_ro.c
> @@ -145,10 +145,10 @@ int fdt_subnode_offset(const void *fdt, int parentoffset,
>   * "foo/bar:option" and "bar:option/otheroption", both of which happen, so
>   * first searching for either ':' or '/' does not work.
>   */
> -static const char *fdt_path_next_separator(const char *path)
> +static const char *fdt_path_next_separator(const char *path, int len)
>  {
> -	const char *sep1 = strchr(path, '/');
> -	const char *sep2 = strchr(path, ':');
> +	const void *sep1 = memchr(path, '/', len);
> +	const void *sep2 = memchr(path, ':', len);
>  
>  	if (sep1 && sep2)
>  		return (sep1 < sep2) ? sep1 : sep2;
> @@ -158,9 +158,9 @@ static const char *fdt_path_next_separator(const char *path)
>  		return sep2;
>  }
>  
> -int fdt_path_offset(const void *fdt, const char *path)
> +int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen)
>  {
> -	const char *end = path + strlen(path);
> +	const char *end = path + namelen;
>  	const char *p = path;
>  	int offset = 0;
>  
> @@ -168,7 +168,7 @@ int fdt_path_offset(const void *fdt, const char *path)
>  
>  	/* see if we have an alias */
>  	if (*path != '/') {
> -		const char *q = fdt_path_next_separator(path);
> +		const char *q = fdt_path_next_separator(path, namelen);
>  
>  		if (!q)
>  			q = end;
> @@ -181,14 +181,16 @@ int fdt_path_offset(const void *fdt, const char *path)
>  		p = q;
>  	}
>  
> -	while (*p) {
> +	while (*p && (p < end)) {

The *p test should now be redundant.  If you do see a \0 within the
length specified by namelen, you should return FDT_ERR_BADPATH, rather
than just truncating at the \0.

... and I just realised that path_offset_namelen() is already
upstream, so I should fix that there too.

So what the hell is this downstream fdt_path_next_separator() nonsense for?

>  		const char *q;
>  
>  		while (*p == '/')
>  			p++;
> +
>  		if (*p == '\0' || *p == ':')
>  			return offset;
> -		q = fdt_path_next_separator(p);
> +
> +		q = fdt_path_next_separator(p, end - p);
>  		if (!q)
>  			q = end;
>
Tom Rini Aug. 21, 2016, 3:07 p.m. UTC | #2
On Tue, Jul 05, 2016 at 10:26:41AM +0200, Maxime Ripard wrote:

> Add a namelen variant of fdt_path_offset to retrieve the node offset using
> only a fixed number of characters.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/include/libfdt.h b/include/libfdt.h
index 4643be5adf39..f13b01f08f71 100644
--- a/include/libfdt.h
+++ b/include/libfdt.h
@@ -365,6 +365,17 @@  int fdt_subnode_offset_namelen(const void *fdt, int parentoffset,
  */
 int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name);
 
+/**
+ * fdt_path_offset_namelen - find a tree node based on substring
+ * @fdt: pointer to the device tree blob
+ * @path: full path of the node to locate
+ * @namelen: number of characters of name to consider
+ *
+ * Identical to fdt_path_offset(), but only examine the first
+ * namelen characters of path for matching the node path.
+ */
+int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen);
+
 /**
  * fdt_path_offset - find a tree node by its full path
  * @fdt: pointer to the device tree blob
@@ -387,7 +398,10 @@  int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name);
  *	-FDT_ERR_BADSTRUCTURE,
  *	-FDT_ERR_TRUNCATED, standard meanings.
  */
-int fdt_path_offset(const void *fdt, const char *path);
+static inline int fdt_path_offset(const void *fdt, const char *path)
+{
+	return fdt_path_offset_namelen(fdt, path, strlen(path));
+}
 
 /**
  * fdt_get_name - retrieve the name of a given node
diff --git a/lib/libfdt/fdt_ro.c b/lib/libfdt/fdt_ro.c
index 6b737b211d2e..9cc98db6e2bf 100644
--- a/lib/libfdt/fdt_ro.c
+++ b/lib/libfdt/fdt_ro.c
@@ -145,10 +145,10 @@  int fdt_subnode_offset(const void *fdt, int parentoffset,
  * "foo/bar:option" and "bar:option/otheroption", both of which happen, so
  * first searching for either ':' or '/' does not work.
  */
-static const char *fdt_path_next_separator(const char *path)
+static const char *fdt_path_next_separator(const char *path, int len)
 {
-	const char *sep1 = strchr(path, '/');
-	const char *sep2 = strchr(path, ':');
+	const void *sep1 = memchr(path, '/', len);
+	const void *sep2 = memchr(path, ':', len);
 
 	if (sep1 && sep2)
 		return (sep1 < sep2) ? sep1 : sep2;
@@ -158,9 +158,9 @@  static const char *fdt_path_next_separator(const char *path)
 		return sep2;
 }
 
-int fdt_path_offset(const void *fdt, const char *path)
+int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen)
 {
-	const char *end = path + strlen(path);
+	const char *end = path + namelen;
 	const char *p = path;
 	int offset = 0;
 
@@ -168,7 +168,7 @@  int fdt_path_offset(const void *fdt, const char *path)
 
 	/* see if we have an alias */
 	if (*path != '/') {
-		const char *q = fdt_path_next_separator(path);
+		const char *q = fdt_path_next_separator(path, namelen);
 
 		if (!q)
 			q = end;
@@ -181,14 +181,16 @@  int fdt_path_offset(const void *fdt, const char *path)
 		p = q;
 	}
 
-	while (*p) {
+	while (*p && (p < end)) {
 		const char *q;
 
 		while (*p == '/')
 			p++;
+
 		if (*p == '\0' || *p == ':')
 			return offset;
-		q = fdt_path_next_separator(p);
+
+		q = fdt_path_next_separator(p, end - p);
 		if (!q)
 			q = end;