[PATCH] libfdt: Add fdt_path_offset_namelen()

[PATCH] libfdt: Add fdt_path_offset_namelen()

[Peter Hurley]

Properties may contain path names which are not NUL-terminated.
For example, the 'stdout-path' property allows the form 'path:options',
where the ':' character terminates the path specifier.

Allow these path names to be used in-place for path descending;
add fdt_path_offset_namelen(), which limits the path name to 'namelen'
characters.

Reimplement fdt_path_offset() as a trivial wrapper.

Signed-off-by: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
---
 libfdt/fdt_ro.c    | 22 ++++++++++++++--------
 libfdt/libfdt.h    | 11 +++++++++++
 libfdt/version.lds |  1 +
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 50007f6..a65e4b5 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -154,9 +154,9 @@ int fdt_subnode_offset(const void *fdt, int parentoffset,
 	return fdt_subnode_offset_namelen(fdt, parentoffset, name, strlen(name));
 }
 
-int fdt_path_offset(const void *fdt, const char *path)
+int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen)
 {
-	const char *end = path + strlen(path);
+	const char *end = path + namelen;
 	const char *p = path;
 	int offset = 0;
 
@@ -164,7 +164,7 @@ int fdt_path_offset(const void *fdt, const char *path)
 
 	/* see if we have an alias */
 	if (*path != '/') {
-		const char *q = strchr(path, '/');
+		const char *q = memchr(path, '/', end - p);
 
 		if (!q)
 			q = end;
@@ -177,14 +177,15 @@ int fdt_path_offset(const void *fdt, const char *path)
 		p = q;
 	}
 
-	while (*p) {
+	while (p < end) {
 		const char *q;
 
-		while (*p == '/')
+		while (*p == '/') {
 			p++;
-		if (! *p)
-			return offset;
-		q = strchr(p, '/');
+			if (p == end)
+				return offset;
+		}
+		q = memchr(p, '/', end - p);
 		if (! q)
 			q = end;
 
@@ -198,6 +199,11 @@ int fdt_path_offset(const void *fdt, const char *path)
 	return offset;
 }
 
+int fdt_path_offset(const void *fdt, const char *path)
+{
+	return fdt_path_offset_namelen(fdt, path, strlen(path));
+}
+
 const char *fdt_get_name(const void *fdt, int nodeoffset, int *len)
 {
 	const struct fdt_node_header *nh = _fdt_offset_ptr(fdt, nodeoffset);
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 02baa84..50b13b4 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -318,6 +318,17 @@ int fdt_subnode_offset_namelen(const void *fdt, int parentoffset,
 int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name);
 
 /**
+ * fdt_path_offset_namelen - find a tree node by its full path
+ * @fdt: pointer to the device tree blob
+ * @path: full path of the node to locate
+ * @namelen: number of characters of path to consider
+ *
+ * Identical to fdt_path_offset(), but only consider the first namelen
+ * characters of path as the path name.
+ */
+int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen);
+
+/**
  * fdt_path_offset - find a tree node by its full path
  * @fdt: pointer to the device tree blob
  * @path: full path of the node to locate
diff --git a/libfdt/version.lds b/libfdt/version.lds
index 80b322b..80e945e 100644
--- a/libfdt/version.lds
+++ b/libfdt/version.lds
@@ -8,6 +8,7 @@ LIBFDT_1.2 {
 		fdt_get_mem_rsv;
 		fdt_subnode_offset_namelen;
 		fdt_subnode_offset;
+		fdt_path_offset_namelen;
 		fdt_path_offset;
 		fdt_get_name;
 		fdt_get_property_namelen;
-- 
2.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libfdt: Add fdt_path_offset_namelen()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On Fri, Mar 06, 2015 at 10:12:38AM -0500, Peter Hurley wrote:
> Properties may contain path names which are not NUL-terminated.
> For example, the 'stdout-path' property allows the form 'path:options',
> where the ':' character terminates the path specifier.
> 
> Allow these path names to be used in-place for path descending;
> add fdt_path_offset_namelen(), which limits the path name to 'namelen'
> characters.
> 
> Reimplement fdt_path_offset() as a trivial wrapper.
> 
> Signed-off-by: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>

I think this function is a good idea, however I would like to see a
testcase for it.

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] libfdt: Add fdt_path_offset_namelen()

[Peter Hurley]

Hi David,

On 03/09/2015 08:17 PM, David Gibson wrote:
> On Fri, Mar 06, 2015 at 10:12:38AM -0500, Peter Hurley wrote:
>> Properties may contain path names which are not NUL-terminated.
>> For example, the 'stdout-path' property allows the form 'path:options',
>> where the ':' character terminates the path specifier.
>>
>> Allow these path names to be used in-place for path descending;
>> add fdt_path_offset_namelen(), which limits the path name to 'namelen'
>> characters.
>>
>> Reimplement fdt_path_offset() as a trivial wrapper.
>>
>> Signed-off-by: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
> 
> I think this function is a good idea, however I would like to see a
> testcase for it.

Sure, I can do that.

I assume you mean a path name with non-NUL termination because
the fdt_path_offset() tests are already exercising the
fdt_path_offset_name() implementation.

Is there a readme somewhere regarding the test matrix (ie.,
which dts files go with which tests)?

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libfdt: Add fdt_path_offset_namelen()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]

On Tue, Mar 10, 2015 at 09:47:44AM -0400, Peter Hurley wrote:
> Hi David,
> 
> On 03/09/2015 08:17 PM, David Gibson wrote:
> > On Fri, Mar 06, 2015 at 10:12:38AM -0500, Peter Hurley wrote:
> >> Properties may contain path names which are not NUL-terminated.
> >> For example, the 'stdout-path' property allows the form 'path:options',
> >> where the ':' character terminates the path specifier.
> >>
> >> Allow these path names to be used in-place for path descending;
> >> add fdt_path_offset_namelen(), which limits the path name to 'namelen'
> >> characters.
> >>
> >> Reimplement fdt_path_offset() as a trivial wrapper.
> >>
> >> Signed-off-by: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
> > 
> > I think this function is a good idea, however I would like to see a
> > testcase for it.
> 
> Sure, I can do that.
> 
> I assume you mean a path name with non-NUL termination because
> the fdt_path_offset() tests are already exercising the
> fdt_path_offset_name() implementation.

Yes, I mean the non-\0-terminated case.  Or more specifically still,
making sure that if you call fdt_path_offset_namelen() on a portion of
a longer path, it correctly gives you the offset for only the partial
path.

That said, there may be some other edge cases that could do with
testing too, if you have time.  In particular I'm thinking of paths
where there are repeated '/' character, and paths ending with one or
more '/' characters.

> Is there a readme somewhere regarding the test matrix (ie.,
> which dts files go with which tests)?

I'm afraid not, apart from the test runner script itself.  I'm not
sure quite what information you're after here.

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] libfdt: Add fdt_path_offset_namelen()

[Peter Hurley]

On 03/13/2015 01:43 AM, David Gibson wrote:
> On Tue, Mar 10, 2015 at 09:47:44AM -0400, Peter Hurley wrote:
>> Hi David,
>>
>> On 03/09/2015 08:17 PM, David Gibson wrote:
>>> On Fri, Mar 06, 2015 at 10:12:38AM -0500, Peter Hurley wrote:
>>>> Properties may contain path names which are not NUL-terminated.
>>>> For example, the 'stdout-path' property allows the form 'path:options',
>>>> where the ':' character terminates the path specifier.
>>>>
>>>> Allow these path names to be used in-place for path descending;
>>>> add fdt_path_offset_namelen(), which limits the path name to 'namelen'
>>>> characters.
>>>>
>>>> Reimplement fdt_path_offset() as a trivial wrapper.
>>>>
>>>> Signed-off-by: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
>>>
>>> I think this function is a good idea, however I would like to see a
>>> testcase for it.
>>
>> Sure, I can do that.
>>
>> I assume you mean a path name with non-NUL termination because
>> the fdt_path_offset() tests are already exercising the
>> fdt_path_offset_name() implementation.
> 
> Yes, I mean the non-\0-terminated case.  Or more specifically still,
> making sure that if you call fdt_path_offset_namelen() on a portion of
> a longer path, it correctly gives you the offset for only the partial
> path.
> 
> That said, there may be some other edge cases that could do with
> testing too, if you have time.  In particular I'm thinking of paths
> where there are repeated '/' character, and paths ending with one or
> more '/' characters.
> 
>> Is there a readme somewhere regarding the test matrix (ie.,
>> which dts files go with which tests)?
> 
> I'm afraid not, apart from the test runner script itself.  I'm not
> sure quite what information you're after here.

Ok, now that I have the test working, I see why you didn't understand
what I was after.

The purpose of fdt_path_offset_namelen() is to perform path-descending
from in-place property values; for example, finding the node offset from
the /chosen/stdout-path property of the form

/ {
	chosen {
		stdout-path = "/ocp/serial@44e04d00:115200";
	};
}

I wrote the test to get string properties containing paths from the
fdt; for example, assuming the fdt contains

	path1 = "/subnode@1/subsubnode:";
	path2 = "/subnode@2/subsubnode@0:";
	path3 = "/subnode@1////subsubnode///:";
	path4 = "/subnode@2///subsubnode///:/subnode@1";

the test did:

	const char *path1;

	path1 = fdt_getprop(fdt, 0, "path1", &len);
	...


I had looked at the run_test.sh scripts and known that there were
multiple dtb files produced for the test_tree1 tests so I was asking
for a handy readme to tell me which dts(i) files to add the string
properties to.

When I got the test running for dtbs produced from test_tree1.body.dtsi,
I discovered there were some cross-referenced tests from other
sources, like sw_tree1.c; thus the changes to test_tree1.body.dtsi
cause other tests to fail.

So I've given up on testing fdt_path_offset_namelen() that way,
and instead, am just using static const paths defined in the test
itself, which I will submit shortly.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html