Fix bitstring allocation on 32-bit platforms

[asomers]

Summary

sys/sys/bitstring.h
Fix a rounding calculation that could undersize a bitstring on
32-bit platforms.

include/bitstring.h
Add sys/param.h for the roundup2 macro

Test Plan

Existing ATF test cases

[gibbs]

I see how this can over allocate on platforms with long > 8bits, but I'm missing how 32bit long platforms are special and can get too little space allocated. Can you provide an example in the checkin comment?

[asomers]

Restore bitstr_size to its originally intended purpose of calculating the
actual number of bytes used by a bitstring, and fix bit_decl to round up
appropriately.

[asomers]

The important thing about division is that you divide.

[gibbs]

If I want to dynamically allocate a bitstr, I use bitstr_size() to tell me how much to allocate. Without the roundup, I'll potentially allocate too little memory.

sys/sys/bitstring.h

109 ↗

(On Diff #17771)

#define bitstr_size(_nbits) (roundup2(_nbits, _BITSTR_BITS) / 8)

128 ↗

(On Diff #17771)

No roundup is required here. Is it clearer to use howmany()? I'm not sure.

[gibbs]

Should we inline roundup2? Prior to this change, at least in user space, bitstring.h only relied on functionality provided in the C standard. It might be nice to keep that.

[asomers]

In D6848#145456, @gibbs wrote:

If I want to dynamically allocate a bitstr, I use bitstr_size() to tell me how much to allocate. Without the roundup, I'll potentially allocate too little memory.

I would say that you should use bit_alloc. But plenty of callers are doing it themselves. I'll do as you suggest, which will basically revert the 2nd diff of this review.

sys/sys/bitstring.h

128 ↗

(On Diff #17771)

Roundup most certainly is required here, unless the roundup is implemented in bitstr_size instead. howmany() is basically just division rounding up.

[gibbs]

sys/sys/bitstring.h

128 ↗

(On Diff #17771)

That's what I meant. If bitstr_size() has to roundup anyway, there's no need to roundup here too.

[asomers]

Move the roundup back into bitstr_size instead of bit_decl

[asomers]

Are you happy with this @gibbs? Due to my vacation, I must commit today or it won't get into 11.0.

[gibbs]

I'm just not sure about the new dependency on sys/param.h. I leave that for you to decide.

[asomers]

Remove sys/param.h include.

[gibbs]

sys/sys/bitstring.h

107–110 ↗

(On Diff #17861)

We don't want to prevent a future declaration of roundup2 in the user's program either. So something like this?

#ifdef roundup2
#define        _bit_roundup2 roundup2
#else
#define        _bit_roundup2(x, y)        (((x)+((y)-1))&(~((y)-1))) /* if y is powers of two */
#endif

Move to private section and update call sites to use the private macro name.

[asomers]

Don't define a publicly visible "roundup2"

[gibbs]

Please fix the extra whitespace and test both with and without a roundup2 defined prior to including the header before landing.

sys/sys/bitstring.h

78 ↗

(On Diff #17862)

Extra newline.