[Intel-wired-lan] [RFC PATCH] intel: Use upper_32_bits and lower_32_bits
Joe Perches
joe at perches.com
Mon Jan 9 13:33:10 UTC 2017
On Mon, 2017-01-09 at 12:55 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 07 January 2017 18:33
> > Shifting and masking various types can be made a bit
> > simpler to read by using the available kernel macros.
>
> ...
> > - ew32(TDBAH, (tdba >> 32));
> > - ew32(TDBAL, (tdba & 0x00000000ffffffffULL));
> > + ew32(TDBAH, upper_32_bits(tdba));
> > + ew32(TDBAL, lower_32_bits(tdba));
>
> Personally I find the original code easier to understand
> since I don't have to look up another silly macro.
It's already a pretty common usage and I believe
the naming is fairly obvious. Also you don't have
to count the "f" characters to see how many bits
are being used. After about 6 consecutive chars,
it can be error prone.
The leading zeros? ugh. The ULL too.
$ git grep -w -E "upper_32_bits|lower_32_bits" | wc -l
1569
> I'd normally not even explicitly mask the low bits
> relying on the implicit truncation of the assignment.
Relying on implicit behaviors can be noisy when
compilers complain about implicit conversions and
truncations.
> At least modern compilers aren't stupid enough to add two
> 'mask with 0xff' instructions for:
> *uchar_ptr = (unsigned char)(foo & 0xff);
I agree it's visual noise.
More information about the Intel-wired-lan
mailing list