lkml.org 
[lkml]   [2014]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v3 2/3] of: add optional options parameter to? of_find_node_by_path()
On Fri, Nov 28, 2014 at 03:25:12PM +0000, Grant Likely wrote:
> On Fri, 28 Nov 2014 11:34:28 +0000
> , Leif Lindholm <leif.lindholm@linaro.org>
> wrote:
> > On Fri, Nov 28, 2014 at 12:44:03AM +0000, Grant Likely wrote:
> > > > + 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.
> >
> > > 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!

And since it's Friday... *cough*

From 5c469dad81961164c444da7d6c4e1c5b1c097aab Mon Sep 17 00:00:00 2001
From: Leif Lindholm <leif.lindholm@linaro.org>
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 <leif.lindholm@linaro.org>
---
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


\
 
 \ /
  Last update: 2014-11-28 18:01    [W:0.085 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site