[PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()

[PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()

[Kumar Gala]

If the path doesn't start with '/' check to see if it matches some alias
under "/aliases" and substitute the matching alias value in the path
and retry the lookup.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---

Simplify down the aliases.dts to only whats needed for the test.

- k

 libfdt/fdt_ro.c             |   21 ++++++++++++++-
 tests/Makefile.tests        |    2 +-
 tests/aliases.dts           |   21 +++++++++++++++
 tests/path_offset_aliases.c |   59 +++++++++++++++++++++++++++++++++++++++++++
 tests/run_tests.sh          |    4 +++
 5 files changed, 104 insertions(+), 3 deletions(-)
 create mode 100644 tests/aliases.dts
 create mode 100644 tests/path_offset_aliases.c

diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index ebd1260..2f3ff48 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -139,8 +139,25 @@ int fdt_path_offset(const void *fdt, const char *path)

 	FDT_CHECK_HEADER(fdt);

-	if (*path != '/')
-		return -FDT_ERR_BADPATH;
+	/* see if we have an alias */
+	if (*path != '/') {
+		const char *q;
+		int aliasoffset = fdt_path_offset(fdt, "/aliases");
+
+		if (aliasoffset < 0)
+			return -FDT_ERR_BADPATH;
+
+		q = strchr(path, '/');
+		if (!q)
+			q = end;
+
+		p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL);
+		if (!p)
+			return -FDT_ERR_BADPATH;
+		offset = fdt_path_offset(fdt, p);
+
+		p = q;
+	}

 	while (*p) {
 		const char *q;
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index 704c95d..44021b0 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -11,7 +11,7 @@ LIB_TESTS_L = get_mem_rsv \
 	open_pack rw_tree1 set_name setprop del_property del_node \
 	string_escapes references path-references boot-cpuid incbin \
 	dtbs_equal_ordered \
-	add_subnode_with_nops
+	add_subnode_with_nops path_offset_aliases
 LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)

 LIBTREE_TESTS_L = truncated_property
diff --git a/tests/aliases.dts b/tests/aliases.dts
new file mode 100644
index 0000000..39d88ff
--- /dev/null
+++ b/tests/aliases.dts
@@ -0,0 +1,21 @@
+/dts-v1/;
+
+/ {
+	aliases {
+		s1 = &sub1;
+		ss1 = &subsub1;
+		sss1 = &subsubsub1;
+	};
+
+	sub1: subnode@1 {
+		compatible = "subnode1";
+
+		subsub1: subsubnode {
+			compatible = "subsubnode1", "subsubnode";
+
+			subsubsub1: subsubsubnode {
+				compatible = "subsubsubnode1", "subsubsubnode";
+			};
+		};
+	};
+};
diff --git a/tests/path_offset_aliases.c b/tests/path_offset_aliases.c
new file mode 100644
index 0000000..191edd2
--- /dev/null
+++ b/tests/path_offset_aliases.c
@@ -0,0 +1,59 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ *	Testcase for fdt_path_offset()
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ * Copyright 2008 Kumar Gala, Freescale Semiconductor, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "tests.h"
+#include "testdata.h"
+
+void check_alias(void *fdt, const char *full_path, const char *alias_path)
+{
+	int offset, offset_a;
+
+	offset = fdt_path_offset(fdt, full_path);
+	offset_a = fdt_path_offset(fdt, alias_path);
+
+	if (offset != offset_a)
+		FAIL("Mismatch between %s path_offset (%d) and %s path_offset alias (%d)",
+		     full_path, offset, alias_path, offset_a);
+}
+
+int main(int argc, char *argv[])
+{
+	void *fdt;
+
+	test_init(argc, argv);
+	fdt = load_blob_arg(argc, argv);
+
+	check_alias(fdt, "/subnode@1", "s1");
+	check_alias(fdt, "/subnode@1/subsubnode", "ss1");
+	check_alias(fdt, "/subnode@1/subsubnode", "s1/subsubnode");
+	check_alias(fdt, "/subnode@1/subsubnode/subsubsubnode", "sss1");
+	check_alias(fdt, "/subnode@1/subsubnode/subsubsubnode", "ss1/subsubsubnode");
+	check_alias(fdt, "/subnode@1/subsubnode/subsubsubnode", "s1/subsubnode/subsubsubnode");
+
+	PASS();
+}
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 7bfc399..eb29462 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -214,6 +214,10 @@ dtc_tests () {
     run_dtc_test -I dts -O dtb -o dtc_comments-cmp.test.dtb comments-cmp.dts
     run_test dtbs_equal_ordered dtc_comments.test.dtb dtc_comments-cmp.test.dtb

+    # Check aliases support in fdt_path_offset
+    run_dtc_test -I dts -O dtb -o aliases.dtb aliases.dts
+    run_test path_offset_aliases aliases.dtb
+
     # Check /include/ directive
     run_dtc_test -I dts -O dtb -o includes.test.dtb include0.dts
     run_test dtbs_equal_ordered includes.test.dtb test_tree1.dtb
-- 
1.5.5.1

Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()

[David Gibson]

On Thu, Aug 14, 2008 at 08:28:19AM -0500, Kumar Gala wrote:
> If the path doesn't start with '/' check to see if it matches some alias
> under "/aliases" and substitute the matching alias value in the path
> and retry the lookup.
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()

[Jon Loeliger]

> If the path doesn't start with '/' check to see if it matches some alias
> under "/aliases" and substitute the matching alias value in the path
> and retry the lookup.
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>

Applied.

jdl

Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()

[Scott Wood]

On Thu, Aug 14, 2008 at 08:28:19AM -0500, Kumar Gala wrote:
> -	if (*path != '/')
> -		return -FDT_ERR_BADPATH;
> +	/* see if we have an alias */
> +	if (*path != '/') {
> +		const char *q;
> +		int aliasoffset = fdt_path_offset(fdt, "/aliases");
> +
> +		if (aliasoffset < 0)
> +			return -FDT_ERR_BADPATH;
> +
> +		q = strchr(path, '/');
> +		if (!q)
> +			q = end;
> +
> +		p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL);
> +		if (!p)
> +			return -FDT_ERR_BADPATH;
> +		offset = fdt_path_offset(fdt, p);
> +
> +		p = q;
> +	}

Can we limit the recursion depth to avoid falling off the stack if an
alias points to itself?  Or if aliases pointing to aliases are
disallowed, check for a leading '/' before recursively calling
fdt_path_offset.

-Scott

Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()

[David Gibson]

On Thu, Aug 14, 2008 at 12:43:48PM -0500, Scott Wood wrote:
> On Thu, Aug 14, 2008 at 08:28:19AM -0500, Kumar Gala wrote:
> > -	if (*path != '/')
> > -		return -FDT_ERR_BADPATH;
> > +	/* see if we have an alias */
> > +	if (*path != '/') {
> > +		const char *q;
> > +		int aliasoffset = fdt_path_offset(fdt, "/aliases");
> > +
> > +		if (aliasoffset < 0)
> > +			return -FDT_ERR_BADPATH;
> > +
> > +		q = strchr(path, '/');
> > +		if (!q)
> > +			q = end;
> > +
> > +		p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL);
> > +		if (!p)
> > +			return -FDT_ERR_BADPATH;
> > +		offset = fdt_path_offset(fdt, p);
> > +
> > +		p = q;
> > +	}
> 
> Can we limit the recursion depth to avoid falling off the stack if an
> alias points to itself?  Or if aliases pointing to aliases are
> disallowed, check for a leading '/' before recursively calling
> fdt_path_offset.

Hmm.. my reading of 1275 says that an alias pointing to another alias
is not permitted, but I'm not terribly confident I'm not misreading
it.  Segher, do you know whether this is allowed?

If that's the case then, yes, we should not recurse if the alias value
doesn't start with a /.  In fact, if I factor out a fdt_get_alias()
function, it should probably check the alias and return an error if
it's not an absolute path.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()

[Mitch Bradley]

David Gibson wrote:
> On Thu, Aug 14, 2008 at 12:43:48PM -0500, Scott Wood wrote:
>   
>> On Thu, Aug 14, 2008 at 08:28:19AM -0500, Kumar Gala wrote:
>>     
>>> -	if (*path != '/')
>>> -		return -FDT_ERR_BADPATH;
>>> +	/* see if we have an alias */
>>> +	if (*path != '/') {
>>> +		const char *q;
>>> +		int aliasoffset = fdt_path_offset(fdt, "/aliases");
>>> +
>>> +		if (aliasoffset < 0)
>>> +			return -FDT_ERR_BADPATH;
>>> +
>>> +		q = strchr(path, '/');
>>> +		if (!q)
>>> +			q = end;
>>> +
>>> +		p = fdt_getprop_namelen(fdt, aliasoffset, path, q - p, NULL);
>>> +		if (!p)
>>> +			return -FDT_ERR_BADPATH;
>>> +		offset = fdt_path_offset(fdt, p);
>>> +
>>> +		p = q;
>>> +	}
>>>       
>> Can we limit the recursion depth to avoid falling off the stack if an
>> alias points to itself?  Or if aliases pointing to aliases are
>> disallowed, check for a leading '/' before recursively calling
>> fdt_path_offset.
>>     
>
> Hmm.. my reading of 1275 says that an alias pointing to another alias
> is not permitted, but I'm not terribly confident I'm not misreading
> it.  Segher, do you know whether this is allowed?
>   

The 1275 spec doesn't require multiple levels of aliasing, but my 
current implementation allows it and uses it for things like components 
of the network stack.


> If that's the case then, yes, we should not recurse if the alias value
> doesn't start with a /.  In fact, if I factor out a fdt_get_alias()
> function, it should probably check the alias and return an error if
> it's not an absolute path.
>
>   

Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()

[Segher Boessenkool]

> Hmm.. my reading of 1275 says that an alias pointing to another alias
> is not permitted, but I'm not terribly confident I'm not misreading
> it.  Segher, do you know whether this is allowed?

My reading is the same: if after expanding an alias the path does
not start with "/", the search starts at the current package,
otherwise it starts at the root node; and no further alias expansion
is done.

It's a corner case for sure, but at least the standard doesn't
require us to implement recursive alias expansion; let's see if
common practice does ;-)


Segher