sysctl_register_oid: fix duplicate oid after d3f96f661050

[dougm]

Summary

sysctl_register_oid must check the uniqueness of any newly computed oid_number in sysctl_register_oid.

[asomers]

It looks like your version will result in more sequential oid numbers. But I don't see how the old version could create a duplicate. Could you please explain?

sys/kern/kern_sysctl.c

497

I like this style better, and it's more searchable too.

[dougm]

In D36743#834231, @asomers wrote:

It looks like your version will result in more sequential oid numbers. But I don't see how the old version could create a duplicate. Could you please explain?

After your change, when the first oid_number tried is found to be in use, you usually increment the oid_number and use it, assuming that it is not also in use. Before your change, when the first oid-number tried is found to be in use, the code usually incremented the old number and iterated, to check whether that number was in use, and kept doing so until it found a number not in use. So, you have introduced a bug, which I am trying to address, by making sure that the oid_number is not in use.

sys/kern/kern_sysctl.c

497

I chose to write this in a way that does not suggest that 'parent' matters, because it does not.

[asomers]

In D36743#834236, @dougm wrote:

In D36743#834231, @asomers wrote:

It looks like your version will result in more sequential oid numbers. But I don't see how the old version could create a duplicate. Could you please explain?

After your change, when the first oid_number tried is found to be in use, you usually increment the oid_number and use it, assuming that it is not also in use. Before your change, when the first oid-number tried is found to be in use, the code usually incremented the old number and iterated, to check whether that number was in use, and kept doing so until it found a number not in use. So, you have introduced a bug, which I am trying to address, by making sure that the oid_number is not in use.

Oh, I see. I missed a goto retry. If you move that out of the if block then that would also fix the problem.

sys/kern/kern_sysctl.c

497

Then what about RB_NEXT(sysctl_oid_list, , p)? I really don't like sysctl_oid_list_RB_NEXT, because when I search through my editor for a regex like \<sysctl_oid_list\> or \<RB_NEXT\>, it won't find it.

[dougm]

In D36743#834240, @asomers wrote:

In D36743#834236, @dougm wrote:

In D36743#834231, @asomers wrote:

It looks like your version will result in more sequential oid numbers. But I don't see how the old version could create a duplicate. Could you please explain?

After your change, when the first oid_number tried is found to be in use, you usually increment the oid_number and use it, assuming that it is not also in use. Before your change, when the first oid-number tried is found to be in use, the code usually incremented the old number and iterated, to check whether that number was in use, and kept doing so until it found a number not in use. So, you have introduced a bug, which I am trying to address, by making sure that the oid_number is not in use.

Oh, I see. I missed a goto retry. If you move that out of the if block then that would also fix the problem.

It would fix the problem, but it would use RB_FIND again, when another tree search isn't really necessary.

[dougm]

Offer an RB_NEXT compromise.

[asomers]

Approved. And even though I probably won't MFC the original, please be sure to add an "MFC With" to your commit message, just in case.

[• hselasky]

You may want to run a script to creating and destroying 2^32 single SYSCTL OIDs to verify the logic.