The fallback for align_up() used by roundup2() uses typeof__()
which doesn't work for bitfields. This fixes the build on GCC which
uses the fallback.
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
LGTM. We could also try the typeof(X+0) approach but if there's only one use that needs fixing adding this one cast seems better than potential unexpected side effects of changing the macro.
One thing to keep in mind is that this macro is visible to userspace, so this may come up again.
The typeof(X+0) approach looks nicer to me. It's hard to imagine what hidden gotchas might arise with that, but we can also revisit it later if the need arises, so the patch seems fine to me.
I think the typeof doesn't work if you use it on a void *, i.e. void *p; roundup2(p, 16). Granted, that code doesn't work without a cast with the old roundup2() either, but this would also mean you would break uses of the newer __align_up() which does support void * in both of its current forms.
Patch itself looks fine.
It's unfortunate that gcc is so picky about this.
It's strange that roundup2() and roundup() have differences in terms of expression types--roundup2() is supposed just to be an optimized roundup(), right? One might think that the next step would be to make them consistent in expression types. Did you happen to take a look at what the fallout of requiring callers to cast would be for roundup()/rounddown()?
I'm not sure how much roundup/roundup2 are going to be used outside of the tree. On Linux other things are used (e.g. DIV_ROUND_UP is like howmany()), so I suspect they aren't really portable and aren't used widely outside of the base system. I haven't tried building world with GCC, but this was the only outlier I found in the kernel. I think if we find more than a handful of places we have to fix, then we will need make roundup2() use its own fallback instead of using __align_up's fallback, but I want to wait to see how many of those we find in practice.