of_getnode_byname: infinite loop

[yarl-baudig%mailoo.org@localhost]

Hello,

of_getnode_byname(start, target) goes on infinite loop if no node with 'target' name exists.

Moreover, I think the function should not go out of the subtree whose 'start' is the root.

Also, both the tests and the documentation can be improved.

I propose the attached patch.

Thank you.

commit 92e06cb016b3a4792f6b6140a7b5055d794420ed
Author: Yarl Baudig <yarl-baudig%mailoo.org@localhost>
Date:   Fri Apr 19 09:24:18 2019 +0200

    Upgrade of_getnode_byname()
    
            -no more infinite loop when no node with 'target' as name exists
            -no more go out of the subtree you are searching in
            -improved (more standard) tests on return values
            -improved documentation

diff --git a/sys/dev/ofw/ofw_subr.c b/sys/dev/ofw/ofw_subr.c
index 91d4b7a004d3..6529c831ef28 100644
--- a/sys/dev/ofw/ofw_subr.c
+++ b/sys/dev/ofw/ofw_subr.c
@@ -337,9 +337,21 @@ of_find_firstchild_byname(int node, const char *name)
 }
 
 /*
- * Find a give node by name.  Recurses, and seems to walk upwards too.
+ * int of_getnode_byname(start, target)
+ *
+ * Find a node by name.
+ *
+ * Arguments:
+ *	start		OFW phandle of the root node of the subtree
+ *			to search in. You can give 0 for the root of the full tree.
+ *	target		Property "name" to search for.
+ *
+ * Return Value:
+ *	-1 error, 0 if no node with such a name exists or the OFW phandle of the first
+ *	node with that name.
+ * Side Effects:
+ *	None.
  */
-
 int
 of_getnode_byname(int start, const char *target)
 {
@@ -351,21 +363,23 @@ of_getnode_byname(int start, const char *target)
 
 	for (node = start; node; node = next) {
 		memset(name, 0, sizeof name);
-		OF_getprop(node, "name", name, sizeof name - 1);
-		if (strcmp(name, target) == 0)
+		if ((OF_getprop(node, "name", name, sizeof name - 1) > 0) &&
+			(strcmp(name, target) == 0))
 			break;
 
-		if ((next = OF_child(node)) != 0)
+		if ((next = OF_child(node)) > 0)
 			continue;
 
 		while (node) {
-			if ((next = OF_peer(node)) != 0)
+			if ((next = OF_peer(node)) > 0)
 				break;
-			node = OF_parent(node);
+			if ((node = OF_parent(node)) < 1)
+				return node;
+			if (node == start)
+				return 0;
 		}
 	}
 
-	/* XXX is this correct? */
 	return node;
 }