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.