Please consider renaming to RB_COUNT(), and please do not change the memory layout or semantics of basic containers which are already used in-kernel (discussed on src-committers@)

I dislike macros like these.

If I would write code against this header, I would expect that this macro has a time complexity that is as efficient as possible. A red-black tree could be augmented to keep track of the count and have an RB_COUNT() that runs in O(1) time. This macro runs in linear time.

For the same reason, I completely dislike SLIST_REMOVE(). We use it all over the place, but it actually has an O(n) running time. Code that uses SLIST_REMOVE() should just be changed to use a LIST instead of an SLIST. SLIST_REMOVE() shouldn't have been added in the first place.

Additional remark: if such a macro is added, it would make more sense to return the count -- not have it passed in by reference.

In D3371#68718, @ed wrote:

I dislike macros like these.

If I would write code against this header, I would expect that this macro has a time complexity that is as efficient as possible. A red-black tree could be augmented to keep track of the count and have an RB_COUNT() that runs in O(1) time. This macro runs in linear time.

I was trying to avoid doing that because it means I would have way more code to do :)

But yes in theory this is better. Actually the initial RB_NUMNODES I wrote was just to minimize conversion from Illumos libavl which has a avl_numnodes (but O(1)) I felt changing the whole tree.h was wrong)

For the same reason, I completely dislike SLIST_REMOVE(). We use it all over the place, but it actually has an O(n) running time. Code that uses SLIST_REMOVE() should just be changed to use a LIST instead of an SLIST. SLIST_REMOVE() shouldn't have been added in the first place.