Why is this licensing tort getting tacked on?

Can this be moved up?

This should be moved down below the licensing/RCS comments.

I'm confused. Why is BSD 2 clause getting tacked on BSD 3 clause?

I actually put these in lib/libc/tests/sys (see queue_test.c). This may or may not be an intuitive location to put these header tests...

@gibbs can you answer this question?

No, because it must succeed stand.h. bcache is weird because it's part of neither the kernel nor userland. sys/bitstring.h requires calloc, which is declared in stand.h. sys/bitstring.h knows how to include the appropriate userland or kernel headers to get calloc, but it doesn't know about stand.h, and bcache's Makefile defines no preprocessor macro to indicate that we are using libstand.

However, Allan Jude just checked in a refactor of bcache.c that totally eliminates bitstring. So I won't actually end up committing this file.

Why there? These aren't really part of libc. I followed the convention of placing the test as close as possible to the code that it tests.

How else would you suggest Spectra Logic document the license governing these changes? Note that the license terms are slightly different from the existing ones (e.g. no clause 3, and clause 2 only requires a substantially similar disclaimer (so disclaimer text can be merged by a distributor) without an individual copyright notice).

This line was to tricky to find. The old code didn't actually initialize the entire bitstr, only the lowest us->len bits.

For the same reason that @gibbs described above. He wished to use the 2 clause license for his changes.

True, but its usefulness will be limited until somebody implements dynamic dispatch for bitcountl in the kernel. Currently, it will only use the popcnt instruction if you compile the kernel with the right -march option. Still, the fallback SWAR approach in sys/types.h should be faster than the naive approach above.

Will do.

Oops. I tried to fix those, but I guess I missed this one. I'll fix it when I commit this revision.

Oy... seems like this should be committed separately?

Missing a newline here before the return.

Missing a newline here before the return.

Missing a newline here before the return.

Missing a newline here before the return.

Missing a newline here before the return.

Should there be a corresponding _free function to wrap around free, such that you don't have to have 2 separate implementations?

Missing a newline here before the return.

Inconsistent braces on single line here and elsewhere.

I did that because it already existed and was the same location that the NetBSD tests live.

This is going to change soon. Stay tuned..

The sorting's off:

sys/param.h
bitstring.h
stdio.h
atf-c.h

int found_clear_bit, i;

?

Why does this need to be commented?

Newline missing here.

• Newline missing here.
• No ellipses around atf_no_error().

I thought I'd find something in style.9 that required a newline in cases of no local variables, but can't find it. An extra newline adds no value to a single line function (no improvement in readability).

The style used here, as in many other places in FreeBSD, is that braces are used if any block in an if/else if/else requires them. Otherwise, a single line block doesn't get them. From my reading of style 9, this is allowed.

If the above style is violated in the file, feel free to flag those instances.

No. The #define was necessary because the old bit_alloc function required calloc. The new bit_alloc function does not. So it's appropriate to commit it together.

Do you want me to move the bitstring tests? I prefer tests/sys/sys for consistency with how we do other tests for code that lives in sys.

Those are the function arguments for the bit_nset_test function. The arguments are defined by the expansion of the BITSTRING_TC_DEFINE macro. Since they're not shown, it's helpful to include the comment.