LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/2] of: fix handling of '/' in path options
@ 2015-03-09 18:03 Leif Lindholm
  2015-03-09 18:03 ` [PATCH 1/2] of: fix handling of '/' in options for of_find_node_by_path() Leif Lindholm
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Leif Lindholm @ 2015-03-09 18:03 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel, linux-kernel; +Cc: grant.likely, robh+dt, peter

Commit 7914a7c5651a ("of: support passing console options with
stdout-path") neglected to deal with '/'s appearing past the ':'
terminator.

This mini-series fixes this oversight and adds the tests to prove it.

Leif Lindholm (1):
  of: fix handling of '/' in options for of_find_node_by_path()

Peter Hurley (1):
  of: unittest: Add options string testcase variants

 drivers/of/base.c     | 23 +++++++++++++++--------
 drivers/of/unittest.c | 11 +++++++++++
 2 files changed, 26 insertions(+), 8 deletions(-)

-- 
2.1.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] of: fix handling of '/' in options for of_find_node_by_path()
  2015-03-09 18:03 [PATCH 0/2] of: fix handling of '/' in path options Leif Lindholm
@ 2015-03-09 18:03 ` Leif Lindholm
  2015-03-17 19:30   ` [PATCH 1/2] of: handle both '/' and ':' in path strings Brian Norris
  2015-03-09 18:03 ` [PATCH 2/2] of: unittest: Add options string testcase variants Leif Lindholm
  2015-03-11 12:49 ` [PATCH 0/2] of: fix handling of '/' in path options Rob Herring
  2 siblings, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2015-03-09 18:03 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel, linux-kernel; +Cc: grant.likely, robh+dt, peter

Ensure proper handling of paths with appended options (after ':'),
where those options may contain a '/'.

Fixes: 7914a7c5651a ("of: support passing console options with stdout-path")
Reported-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 drivers/of/base.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 0a8aeb8..8b904e5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -714,16 +714,17 @@ 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;
-	int term;
+	int len;
+	const char *end;
 
+	end = strchr(path, ':');
+	if (!end)
+		end = strchrnul(path, '/');
+
+	len = end - path;
 	if (!len)
 		return NULL;
 
-	term = strchrnul(path, ':') - path;
-	if (term < len)
-		len = term;
-
 	__for_each_child_of_node(parent, child) {
 		const char *name = strrchr(child->full_name, '/');
 		if (WARN(!name, "malformed device_node %s\n", child->full_name))
@@ -768,8 +769,12 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
 
 	/* The path could begin with an alias */
 	if (*path != '/') {
-		char *p = strchrnul(path, '/');
-		int len = separator ? separator - path : p - path;
+		int len;
+		const char *p = separator;
+
+		if (!p)
+			p = strchrnul(path, '/');
+		len = p - path;
 
 		/* of_aliases must not be NULL */
 		if (!of_aliases)
@@ -794,6 +799,8 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
 		path++; /* Increment past '/' delimiter */
 		np = __of_find_node_by_path(np, path);
 		path = strchrnul(path, '/');
+		if (separator && separator < path)
+			break;
 	}
 	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 	return np;
-- 
2.1.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] of: unittest: Add options string testcase variants
  2015-03-09 18:03 [PATCH 0/2] of: fix handling of '/' in path options Leif Lindholm
  2015-03-09 18:03 ` [PATCH 1/2] of: fix handling of '/' in options for of_find_node_by_path() Leif Lindholm
@ 2015-03-09 18:03 ` Leif Lindholm
  2015-03-11 12:49 ` [PATCH 0/2] of: fix handling of '/' in path options Rob Herring
  2 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2015-03-09 18:03 UTC (permalink / raw)
  To: devicetree, linux-arm-kernel, linux-kernel; +Cc: grant.likely, robh+dt, peter

From: Peter Hurley <peter@hurleysoftware.com>

Add testcase variants with '/' in the options string to test for
scan beyond end path name terminated by ':'.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/of/unittest.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 0cf9a23..b2d7547 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -92,6 +92,11 @@ static void __init of_selftest_find_node_by_name(void)
 		 "option path test failed\n");
 	of_node_put(np);
 
+	np = of_find_node_opts_by_path("/testcase-data:test/option", &options);
+	selftest(np && !strcmp("test/option", options),
+		 "option path test, subcase #1 failed\n");
+	of_node_put(np);
+
 	np = of_find_node_opts_by_path("/testcase-data:testoption", NULL);
 	selftest(np, "NULL option path test failed\n");
 	of_node_put(np);
@@ -102,6 +107,12 @@ static void __init of_selftest_find_node_by_name(void)
 		 "option alias path test failed\n");
 	of_node_put(np);
 
+	np = of_find_node_opts_by_path("testcase-alias:test/alias/option",
+				       &options);
+	selftest(np && !strcmp("test/alias/option", options),
+		 "option alias path test, subcase #1 failed\n");
+	of_node_put(np);
+
 	np = of_find_node_opts_by_path("testcase-alias:testaliasoption", NULL);
 	selftest(np, "NULL option alias path test failed\n");
 	of_node_put(np);
-- 
2.1.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] of: fix handling of '/' in path options
  2015-03-09 18:03 [PATCH 0/2] of: fix handling of '/' in path options Leif Lindholm
  2015-03-09 18:03 ` [PATCH 1/2] of: fix handling of '/' in options for of_find_node_by_path() Leif Lindholm
  2015-03-09 18:03 ` [PATCH 2/2] of: unittest: Add options string testcase variants Leif Lindholm
@ 2015-03-11 12:49 ` Rob Herring
  2015-03-11 13:00   ` Leif Lindholm
  2 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2015-03-11 12:49 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: devicetree, linux-arm-kernel, linux-kernel, Grant Likely,
	Rob Herring, Peter Hurley

On Mon, Mar 9, 2015 at 1:03 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> Commit 7914a7c5651a ("of: support passing console options with
> stdout-path") neglected to deal with '/'s appearing past the ':'
> terminator.
>
> This mini-series fixes this oversight and adds the tests to prove it.
>
> Leif Lindholm (1):
>   of: fix handling of '/' in options for of_find_node_by_path()
>
> Peter Hurley (1):
>   of: unittest: Add options string testcase variants

Are there changes from the previous versions? I've already pulled those in.

Rob

>
>  drivers/of/base.c     | 23 +++++++++++++++--------
>  drivers/of/unittest.c | 11 +++++++++++
>  2 files changed, 26 insertions(+), 8 deletions(-)
>
> --
> 2.1.4
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] of: fix handling of '/' in path options
  2015-03-11 12:49 ` [PATCH 0/2] of: fix handling of '/' in path options Rob Herring
@ 2015-03-11 13:00   ` Leif Lindholm
  0 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2015-03-11 13:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, linux-arm-kernel, linux-kernel, Grant Likely,
	Rob Herring, Peter Hurley

On Wed, Mar 11, 2015 at 07:49:33AM -0500, Rob Herring wrote:
> On Mon, Mar 9, 2015 at 1:03 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > Commit 7914a7c5651a ("of: support passing console options with
> > stdout-path") neglected to deal with '/'s appearing past the ':'
> > terminator.
> >
> > This mini-series fixes this oversight and adds the tests to prove it.
> >
> > Leif Lindholm (1):
> >   of: fix handling of '/' in options for of_find_node_by_path()
> >
> > Peter Hurley (1):
> >   of: unittest: Add options string testcase variants
> 
> Are there changes from the previous versions? I've already pulled those in.

No, just sent together as a series since I didn't realise you had.

/
    Leif

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] of: handle both '/' and ':' in path strings
  2015-03-09 18:03 ` [PATCH 1/2] of: fix handling of '/' in options for of_find_node_by_path() Leif Lindholm
@ 2015-03-17 19:30   ` Brian Norris
  2015-03-17 19:30     ` [PATCH 2/2] of: unittest: Add option string test case with longer path Brian Norris
                       ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Brian Norris @ 2015-03-17 19:30 UTC (permalink / raw)
  To: Rob Herring
  Cc: Brian Norris, Peter Hurley, Leif Lindholm, stable, Grant Likely,
	Rob Herring, devicetree, linux-kernel

Commit 106937e8ccdc ("of: fix handling of '/' in options for
of_find_node_by_path()") caused a regression in OF handling of
stdout-path. While it fixes some cases which have '/' after the ':', it
breaks cases where there is more than one '/' *before* the ':'.

For example, it breaks this boot string

  stdout-path = "/rdb/serial@f040ab00:115200";

So rather than doing sequentialized checks (first for '/', then for ':';
or vice versa), to get the correct behavior we need to check for the
first occurrence of either one of them.

It so happens that the handy strcspn() helper can do just that.

Fixes: 106937e8ccdc ("of: fix handling of '/' in options for of_find_node_by_path()")
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: stable@vger.kernel.org
---
This is for -stable only because the regression is marked for stable. Not sure
the first one deserves to go to -stable, actually...

 drivers/of/base.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index adb8764861c0..966d6fdcf427 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -715,13 +715,8 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
 {
 	struct device_node *child;
 	int len;
-	const char *end;
 
-	end = strchr(path, ':');
-	if (!end)
-		end = strchrnul(path, '/');
-
-	len = end - path;
+	len = strcspn(path, "/:");
 	if (!len)
 		return NULL;
 
-- 
1.9.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] of: unittest: Add option string test case with longer path
  2015-03-17 19:30   ` [PATCH 1/2] of: handle both '/' and ':' in path strings Brian Norris
@ 2015-03-17 19:30     ` Brian Norris
  2015-03-17 21:12       ` Leif Lindholm
  2015-03-17 21:11     ` [PATCH 1/2] of: handle both '/' and ':' in path strings Leif Lindholm
  2015-03-22 20:04     ` Rob Herring
  2 siblings, 1 reply; 10+ messages in thread
From: Brian Norris @ 2015-03-17 19:30 UTC (permalink / raw)
  To: Rob Herring
  Cc: Brian Norris, Peter Hurley, Leif Lindholm, stable, Grant Likely,
	Rob Herring, devicetree, linux-kernel

There were regressions seen with commit 106937e8ccdc ("of: fix handling
of '/' in options for of_find_node_by_path()"), where we couldn't handle
extra '/' before the ':'. Let's test for this now.

Confirmed that this test fails without the previous patch and passes
when patched. All other tests pass.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/of/unittest.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index aba8946cac46..52c45c7df07f 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -97,6 +97,11 @@ static void __init of_selftest_find_node_by_name(void)
 		 "option path test, subcase #1 failed\n");
 	of_node_put(np);
 
+	np = of_find_node_opts_by_path("/testcase-data/testcase-device1:test/option", &options);
+	selftest(np && !strcmp("test/option", options),
+		 "option path test, subcase #2 failed\n");
+	of_node_put(np);
+
 	np = of_find_node_opts_by_path("/testcase-data:testoption", NULL);
 	selftest(np, "NULL option path test failed\n");
 	of_node_put(np);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] of: handle both '/' and ':' in path strings
  2015-03-17 19:30   ` [PATCH 1/2] of: handle both '/' and ':' in path strings Brian Norris
  2015-03-17 19:30     ` [PATCH 2/2] of: unittest: Add option string test case with longer path Brian Norris
@ 2015-03-17 21:11     ` Leif Lindholm
  2015-03-22 20:04     ` Rob Herring
  2 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2015-03-17 21:11 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rob Herring, Peter Hurley, stable, Grant Likely, Rob Herring,
	devicetree, linux-kernel

On Tue, Mar 17, 2015 at 12:30:31PM -0700, Brian Norris wrote:
> Commit 106937e8ccdc ("of: fix handling of '/' in options for
> of_find_node_by_path()") caused a regression in OF handling of
> stdout-path. While it fixes some cases which have '/' after the ':', it
> breaks cases where there is more than one '/' *before* the ':'.
> 
> For example, it breaks this boot string
> 
>   stdout-path = "/rdb/serial@f040ab00:115200";
> 
> So rather than doing sequentialized checks (first for '/', then for ':';
> or vice versa), to get the correct behavior we need to check for the
> first occurrence of either one of them.
> 
> It so happens that the handy strcspn() helper can do just that.
> 
> Fixes: 106937e8ccdc ("of: fix handling of '/' in options for of_find_node_by_path()")
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: stable@vger.kernel.org
> ---
> This is for -stable only because the regression is marked for stable. Not sure
> the first one deserves to go to -stable, actually...
> 
>  drivers/of/base.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index adb8764861c0..966d6fdcf427 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -715,13 +715,8 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
>  {
>  	struct device_node *child;
>  	int len;
> -	const char *end;
>  
> -	end = strchr(path, ':');
> -	if (!end)
> -		end = strchrnul(path, '/');
> -
> -	len = end - path;
> +	len = strcspn(path, "/:");
>  	if (!len)
>  		return NULL;
>  
> -- 
> 1.9.1

Yeah, that's neater that the fix I sent out earlier today.

Acked-by: Leif Lindholm <leif.lindholm@linaro.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] of: unittest: Add option string test case with longer path
  2015-03-17 19:30     ` [PATCH 2/2] of: unittest: Add option string test case with longer path Brian Norris
@ 2015-03-17 21:12       ` Leif Lindholm
  0 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2015-03-17 21:12 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rob Herring, Peter Hurley, stable, Grant Likely, Rob Herring,
	devicetree, linux-kernel

On Tue, Mar 17, 2015 at 12:30:32PM -0700, Brian Norris wrote:
> There were regressions seen with commit 106937e8ccdc ("of: fix handling
> of '/' in options for of_find_node_by_path()"), where we couldn't handle
> extra '/' before the ':'. Let's test for this now.
> 
> Confirmed that this test fails without the previous patch and passes
> when patched. All other tests pass.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/of/unittest.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index aba8946cac46..52c45c7df07f 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -97,6 +97,11 @@ static void __init of_selftest_find_node_by_name(void)
>  		 "option path test, subcase #1 failed\n");
>  	of_node_put(np);
>  
> +	np = of_find_node_opts_by_path("/testcase-data/testcase-device1:test/option", &options);
> +	selftest(np && !strcmp("test/option", options),
> +		 "option path test, subcase #2 failed\n");
> +	of_node_put(np);
> +
>  	np = of_find_node_opts_by_path("/testcase-data:testoption", NULL);
>  	selftest(np, "NULL option path test failed\n");
>  	of_node_put(np);
> -- 
> 1.9.1

Acked-by: Leif Lindholm <leif.lindholm@linaro.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] of: handle both '/' and ':' in path strings
  2015-03-17 19:30   ` [PATCH 1/2] of: handle both '/' and ':' in path strings Brian Norris
  2015-03-17 19:30     ` [PATCH 2/2] of: unittest: Add option string test case with longer path Brian Norris
  2015-03-17 21:11     ` [PATCH 1/2] of: handle both '/' and ':' in path strings Leif Lindholm
@ 2015-03-22 20:04     ` Rob Herring
  2 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2015-03-22 20:04 UTC (permalink / raw)
  To: Brian Norris
  Cc: Peter Hurley, Leif Lindholm, stable, Grant Likely, Rob Herring,
	devicetree, linux-kernel

On Tue, Mar 17, 2015 at 2:30 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> Commit 106937e8ccdc ("of: fix handling of '/' in options for
> of_find_node_by_path()") caused a regression in OF handling of
> stdout-path. While it fixes some cases which have '/' after the ':', it
> breaks cases where there is more than one '/' *before* the ':'.
>
> For example, it breaks this boot string
>
>   stdout-path = "/rdb/serial@f040ab00:115200";
>
> So rather than doing sequentialized checks (first for '/', then for ':';
> or vice versa), to get the correct behavior we need to check for the
> first occurrence of either one of them.
>
> It so happens that the handy strcspn() helper can do just that.
>
> Fixes: 106937e8ccdc ("of: fix handling of '/' in options for of_find_node_by_path()")
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: stable@vger.kernel.org

Thanks. Applied both and in Linus' tree now.

Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-03-22 20:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-09 18:03 [PATCH 0/2] of: fix handling of '/' in path options Leif Lindholm
2015-03-09 18:03 ` [PATCH 1/2] of: fix handling of '/' in options for of_find_node_by_path() Leif Lindholm
2015-03-17 19:30   ` [PATCH 1/2] of: handle both '/' and ':' in path strings Brian Norris
2015-03-17 19:30     ` [PATCH 2/2] of: unittest: Add option string test case with longer path Brian Norris
2015-03-17 21:12       ` Leif Lindholm
2015-03-17 21:11     ` [PATCH 1/2] of: handle both '/' and ':' in path strings Leif Lindholm
2015-03-22 20:04     ` Rob Herring
2015-03-09 18:03 ` [PATCH 2/2] of: unittest: Add options string testcase variants Leif Lindholm
2015-03-11 12:49 ` [PATCH 0/2] of: fix handling of '/' in path options Rob Herring
2015-03-11 13:00   ` Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).