Substantially reduce the number of signed/unsigned issues (warnings if
enabled). While these are presently disabled for FreeBSD, being able to
enable another warning would be good.
Details
- Reviewers
kib jhb se jhibbits - Commits
- rG99adf661ab63: sys: declare bit sets unsigned
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
This needs someone knowledgeable to look and some testing. I imagine most, if not all, uses of bit sets act as if unsigned; if any do not that would be a problem.
This is specifically targeting the CPU_FOREACH(i) macro. The current definition is:
#define CPU_FOREACH(i) \ for ((i) = 0; (i) <= mp_maxid; (i)++) \ if (!CPU_ABSENT((i)))
Trick with this is mp_maxid is a u_int which means the variable should be unsigned, otherwise you would get a warning about signed/unsigned comparison. Whereas since CPU_ABSENT(i) uses a bit set, unless the variable is signed, you get a warning about signed/unsigned conversion.
As a result without this, if -Wsign-conversion and -Wsign-compare are enabled every use of CPU_FOREACH() will result in a warning. With this, if the variable is unsigned there won't be any warnings.
Trying for more reviewers since D32793 has simply been sitting out here gathering dust. Seems pretty appropriate as unsigned is by far the more common use.
I suspect unsigned is also better? Maybe @jrtc27 has thoughts? This is probably a user-facing API change since it affects cpuset_t in userland and possibly some other things.
Plain unsigned would change alignment and thus potentially ABI if someone's embedding this in a bigger structure. Changing from long to unsigned long should be fine as it's a private member and the representation isn't changing (and if you get crazy with LTO across this change I believe it should still be fine given long and unsigned long can alias).