[PATCH v3 2/3] of: add optional options parameter to? of_find_node_by_path()
From: Grant Likely <hidden>
Date: 2014-11-28 23:58:12
Also in:
linux-devicetree, lkml
On Fri, Nov 28, 2014 at 4:38 PM, Leif Lindholm [off-list ref] wrote:
On Fri, Nov 28, 2014 at 03:25:12PM +0000, Grant Likely wrote:quoted
On Fri, 28 Nov 2014 11:34:28 +0000 , Leif Lindholm [off-list ref] wrote:quoted
On Fri, Nov 28, 2014 at 12:44:03AM +0000, Grant Likely wrote:quoted
quoted
+ separator = strchr(path, ':'); + if (separator && opts) + *opts = separator + 1; +What about when there are no opts? Do we require the caller to make sure opts is NULL before calling the function (which sounds like a good source of bugs) or do we clear it on successful return? I think if opts is passed in, but there are no options, then it should set *opts = NULL.Yeah, oops.quoted
There should be test cases for this also. Must set opts to NULL on successful return, and (I think) should leave opts alone on an unsuccessful search.I would actually argue for always nuking the opts - since that could (theoretically) prevent something working by accident in spite of error conditions. How about the below?Perfect, applied with one fixup below...Thanks!
Fixups merged. Thanks. g.
quoted hunk ↗ jump to hunk
And since it's Friday... *cough* From 5c469dad81961164c444da7d6c4e1c5b1c097aab Mon Sep 17 00:00:00 2001 From: Leif Lindholm <redacted> Date: Fri, 28 Nov 2014 16:27:25 +0000 Subject: [PATCH] of: fix options clearing when path is "/" The addition of the optional options extraction on of_find_node_opts_by_path failed to clear incoming options pointer if the specified path was "/". Resolve this case, and add a test to detect it. Signed-off-by: Leif Lindholm <redacted> --- drivers/of/base.c | 11 ++++++----- drivers/of/selftest.c | 5 +++++ 2 files changed, 11 insertions(+), 5 deletions(-)diff --git a/drivers/of/base.c b/drivers/of/base.c index 5e16c6b..32664d1 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c@@ -743,15 +743,16 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt struct device_node *np = NULL; struct property *pp; unsigned long flags; - char *separator; + char *separator = NULL; + + if (opts) { + separator = strchr(path, ':'); + *opts = separator ? separator + 1 : NULL; + } if (strcmp(path, "/") == 0) return of_node_get(of_allnodes); - separator = strchr(path, ':'); - if (opts) - *opts = separator ? separator + 1 : NULL; - /* The path could begin with an alias */ if (*path != '/') { char *p = strchrnul(path, '/');diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c index 721b2a4..914f0ae 100644 --- a/drivers/of/selftest.c +++ b/drivers/of/selftest.c@@ -100,6 +100,11 @@ static void __init of_selftest_find_node_by_name(void) np = of_find_node_opts_by_path("testcase-alias", &options); selftest(np && !options, "option clearing test failed\n"); of_node_put(np); + + options = "testoption"; + np = of_find_node_opts_by_path("/", &options); + selftest(np && !options, "option clearing root node test failed\n"); + of_node_put(np); } static void __init of_selftest_dynamic(void) --1.7.10.4