Move inner loop logic out of sysctl_sysctl_next_ls()

[melifaro]

Summary

Refactor sysctl_sysctl_next_ls().

• Move huge inner loop out of sysctl_sysctl_next_ls() into a separate non-recursive function, returning the next step to be taken.
• Update resulting node oid parts only on successful lookup
• Make sysctl_sysctl_next_ls() return boolean success/failure instead of errno, slightly simplifying logic

Test Plan

7:40 [1] m@devel0 sysctl -aN | wc -l
   13333
7:41 [1] m@devel0 wc -l ~/devel0_stock.txt
   13333 /home/melifaro/devel0_stock.txt

[melifaro]

Update.

[melifaro]

update,

[melifaro]

update wording.

[melifaro]

update.

[melifaro]

Further simplify.

[freqlabs]

I like that deciding what action to take at the current node is able to be split out into its own function. I don't find sysctl_sysctl_next_node to be a properly descriptive name. It kept confusing me that the "next node" function doesn't do what it sounds like. Changing that to something like sysctl_sysctl_next_action and using an enum for the different actions would avoid the confusion. The variable its return value is stored in could be called action instead of ret. Consider for example enum { ITER_STOP, ITER_SIBLINGS, ITER_CHILDREN };.

sys/kern/kern_sysctl.c

1241 ↗

(On Diff #79003)

You could return (success); to shave off the extra return above.

[melifaro]

Address freqlabs comments, rebase to latest HEAD.

[melifaro]

In D27029#604261, @freqlabs wrote:

I like that deciding what action to take at the current node is able to be split out into its own function. I don't find sysctl_sysctl_next_node to be a properly descriptive name. It kept confusing me that the "next node" function doesn't do what it sounds like. Changing that to something like sysctl_sysctl_next_action and using an enum for the different actions would avoid the confusion. The variable its return value is stored in could be called action instead of ret. Consider for example enum { ITER_STOP, ITER_SIBLINGS, ITER_CHILDREN };.

Thank you for the suggestion, that's a good one!
Does the current code look better?

[freqlabs]

sys/kern/kern_sysctl.c

1205 ↗

(On Diff #79531)

ITER_SIBLINGS would be a better fit than ITER_STOP. We aren't stopping iteration, we're continuing to iterate through the siblings of this node.

1210 ↗

(On Diff #79531)

Worth an assert IMO.

[melifaro]

Address freqlab comments.

[freqlabs]

sys/kern/kern_sysctl.c

1104 ↗

(On Diff #79626)

[melifaro]

Update ITER_SIBLINGS comment.

[freqlabs]

sys/kern/kern_sysctl.c

1182 ↗

(On Diff #79628)

Actually I think this should be ITER_SIBLINGS, and below as well, to match the continues that were previously here.

[freqlabs]

sys/kern/kern_sysctl.c

1180 ↗

(On Diff #79628)

In other words, we have a match at the current level but not a complete match.

[melifaro]

Update to address freqlabs comments.