git: e96ae5cb05b3 - main - sysctl_search_oid: remove useless tests

From: Doug Moore <dougm_at_FreeBSD.org>
Date: Tue, 27 Sep 2022 18:33:12 UTC
The branch main has been updated by dougm:

URL: https://cgit.FreeBSD.org/src/commit/?id=e96ae5cb05b3138e106b42649aca2fed71c7fc78

commit e96ae5cb05b3138e106b42649aca2fed71c7fc78
Author:     Doug Moore <dougm@FreeBSD.org>
AuthorDate: 2022-09-27 18:30:31 +0000
Commit:     Doug Moore <dougm@FreeBSD.org>
CommitDate: 2022-09-27 18:30:31 +0000

    sysctl_search_oid: remove useless tests
    
    sysctl_search_old makes several tests in a loop that can be removed.
    
    The first test in the loop is only ever true on the first loop
    iteration, and is always true on that iteration, so its work can be
    done before the loop begins.
    
    The upper and lower bounds on the loop variable 'indx' are each tested
    on each iteration, but 'indx' is changed in one direction or the other
    only once within the loop, so only one bound needs to be checked.
    
    Two ways remain in the loop that nodes[indx] can change (after one of
    them is put before the loop start), and one of them applies exactly
    when indx has been incremented, so no separate test for that case
    requires testing.
    
    Restructure and add comments that makes clearer that this is a basic
    depth-first search.
    
    Reviewed by:    hselasky
    Differential Revision:  https://reviews.freebsd.org/D36741
---
 sys/kern/kern_sysctl.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
index a31c8f97daa1..a655e9e50677 100644
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -356,29 +356,34 @@ sysctl_search_oid(struct sysctl_oid **nodes, struct sysctl_oid *needle)
 
 	SYSCTL_ASSERT_LOCKED();
 	indx = 0;
-	while (indx < CTL_MAXNAME && indx >= 0) {
-		if (nodes[indx] == NULL && indx == 0)
-			nodes[indx] = RB_MIN(sysctl_oid_list,
-			    &sysctl__children);
-		else if (nodes[indx] == NULL)
-			nodes[indx] = RB_MIN(sysctl_oid_list,
-			    &nodes[indx - 1]->oid_children);
-		else
-			nodes[indx] = RB_NEXT(sysctl_oid_list,
-			    &nodes[indx - 1]->oid_children, nodes[indx]);
-
+	/*
+	 * Do a depth-first search of the oid tree, looking for 'needle'. Start
+	 * with the first child of the root.
+	 */
+	nodes[indx] = RB_MIN(sysctl_oid_list, &sysctl__children);
+	for (;;) {
 		if (nodes[indx] == needle)
 			return (indx + 1);
 
 		if (nodes[indx] == NULL) {
-			indx--;
-			continue;
-		}
-
-		if ((nodes[indx]->oid_kind & CTLTYPE) == CTLTYPE_NODE) {
-			indx++;
+			/* Node has no more siblings, so back up to parent. */
+			if (indx-- == 0) {
+				/* Retreat to root, so give up. */
+				break;
+			}
+		} else if ((nodes[indx]->oid_kind & CTLTYPE) == CTLTYPE_NODE) {
+			/* Node has children. */
+			if (++indx == CTL_MAXNAME) {
+				/* Max search depth reached, so give up. */
+				break;
+			}
+			/* Start with the first child. */
+			nodes[indx] = RB_MIN(sysctl_oid_list,
+			    &nodes[indx - 1]->oid_children);
 			continue;
 		}
+		/* Consider next sibling. */
+		nodes[indx] = RB_NEXT(sysctl_oid_list, NULL, nodes[indx]);
 	}
 	return (-1);
 }