Thread (19 messages) 19 messages, 2 authors, 2014-05-23

Re: [PATCH 2/3] of: Make of_find_node_by_path() handle /aliases

From: Grant Likely <hidden>
Date: 2014-05-23 21:22:36
Also in: lkml

On Thu, 22 May 2014 18:14:38 -0700, Frank Rowand [off-list ref] wrote:
On 5/21/2014 6:16 PM, Grant Likely wrote:
quoted
On Tue, 20 May 2014 19:41:22 -0700, Frank Rowand [off-list ref] wrote:
quoted
On 5/18/2014 2:27 AM, Grant Likely wrote:
quoted
On Fri, 16 May 2014 11:54:44 +0100, Grant Likely [off-list ref] wrote:
quoted
On Thu, 15 May 2014 19:51:17 -0700, Frank Rowand [off-list ref] wrote:
quoted
On 5/13/2014 7:58 AM, Grant Likely wrote:
quoted
Make of_find_node_by_path() handle aliases as prefixes. To make this
work the name search is refactored to search by path component instead
of by full string. This should be a more efficient search, and it makes
it possible to start a search at a subnode of a tree.

Signed-off-by: David Daney <redacted>
Signed-off-by: Pantelis Antoniou <redacted>
[grant.likely: Rework to not require allocating at runtime]
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Grant Likely <redacted>
---
 drivers/of/base.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 4 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6e240698353b..60089b9a3014 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -771,9 +771,38 @@ struct device_node *of_get_child_by_name(const struct device_node *node,
 }
 EXPORT_SYMBOL(of_get_child_by_name);
 
+static struct device_node *__of_find_node_by_path(struct device_node *parent,
+						const char *path)
+{
+	struct device_node *child;
+	int len = strchrnul(path, '/') - path;
+
+	if (!len)
+		return parent;
(!len) is true if the the final character of the path passed into of_find_node_by_path()
was "/".  Strictly speaking, ->full_name will never end with "/", so the return value
should be NULL, indicating that the match fails.
Ah, good catch. I should add a test case for that.
In my testing this looks okay. The while loop that calls into
__of_find_node_by_path() looks like this:

	while (np && *path == '/') {
		path++; /* Increment past '/' delimiter */
		np = __of_find_node_by_path(np, path);
		path = strchrnul(path, '/');
	}

If the path ends with a '/', then the loop will go around one more time.
The pointer will be incremented to point at the null character and len
will be null because strchrnul() will point at the last item.
Yes, that was my point.  The old version of of_find_node_by_path() would not
find a match if the path ended with a "/" (unless the full path was "/").
This patch series changes the behavior to be a match.

I will reply to this email with an additional patch that restores the
original behavior.

If you move the additional test cases you provide below and the test cases
in patch 3 to the beginning of the series, you can see the before and after
behavior of adding patch 1 and patch 2.
Ah, I see. That raises the question about what the behaviour /should/
be. Off the top of my head, matching against a trailing '/' seems to be
okay. Are there situations that you see or can think of where matching
would be the wrong thing to do?
I have not thought of a case where matching against a trailing '/' would
hurt anything.  It just seemed better to be consistent in naming.
I've gone ahead and merged in the trailing '/' fix. It can be relaxed
later if deemed important.

g.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help