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

2008-08-14 Thread David Gibson
On Thu, Aug 14, 2008 at 07:51:47AM -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 <[EMAIL PROTECTED]>
> ---
> 
> Fixed the bug pointed out by David Gibson and added tests.

Glad you spotted it in the end :)

[snip[]
> 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;
> + }

Much better.  It would be quite nice to have an explicit way of
retreiving the aliases too, but I can factor that out easily enough in
a later patch.

[snip]
> --- /dev/null
> +++ b/tests/aliases.dts
> @@ -0,0 +1,31 @@
> +/dts-v1/;
> +
> +/memreserve/ 0xdeadbeef 0x10;
> +/memreserve/ 123456789 01;

I'd drop these /memreserve/s, they're not relevant to the test.

> +
> +/ {
> + compatible = "test_tree1";
> + prop-int = <0xdeadbeef>;
> + prop-str = "hello world";

Likewise the various prop-int and prop-str properties.

-- 
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
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2008-08-14 Thread 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 <[EMAIL PROTECTED]>
---

Fixed the bug pointed out by David Gibson and added tests.

- k

 libfdt/fdt_ro.c |   21 ++-
 tests/Makefile.tests|2 +-
 tests/aliases.dts   |   31 ++
 tests/path_offset_aliases.c |   59 +++
 tests/run_tests.sh  |4 +++
 5 files changed, 114 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 000..bd0e6bf
--- /dev/null
+++ b/tests/aliases.dts
@@ -0,0 +1,31 @@
+/dts-v1/;
+
+/memreserve/ 0xdeadbeef 0x10;
+/memreserve/ 123456789 01;
+
+/ {
+   compatible = "test_tree1";
+   prop-int = <0xdeadbeef>;
+   prop-str = "hello world";
+
+   aliases {
+   s1 = &sub1;
+   ss1 = &subsub1;
+   sss1 = &subsubsub1;
+   };
+
+   sub1: [EMAIL PROTECTED] {
+   compatible = "subnode1";
+   prop-int = [deadbeef];
+
+   subsub1: subsubnode {
+   compatible = "subsubnode1", "subsubnode";
+   prop-int = <0xdeadbeef>;
+
+   subsubsub1: subsubsubnode {
+   compatible = "subsubsubnode1", "subsubsubnode";
+   prop-int = <0xdeadbeef>;
+   };
+   };
+   };
+};
diff --git a/tests/path_offset_aliases.c b/tests/path_offset_aliases.c
new file mode 100644
index 000..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 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#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, "/[EMAIL PROTECTED]", "s1");
+