Re: [PATCH] staging: rtl8723bs: Replace `& 0xfff` with `% 4096u`
Greg KH <[email protected]> Mon, 14 Apr 2025 09:21:28 +0200
| Newsgroups | dev.linux.lists.outreachy,dev.linux.lists.linux-staging,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <2025041402-colonize-rocker-1db1@gregkh> |
On Wed, Apr 09, 2025 at 06:48:02PM +0000, Abraham Samuel Adekunle wrote: > The sequence number is constrained to a range of [0, 4095], which > is a total of 4096 values. The bitmask operation using `& 0xfff` is > used to perform this wrap-around. While this is functionally correct, > it obscures the intended semantic of a 4096-based wrap. > > Using a modulo operation `% 4096u` makes the wrap-around logic > explicit and easier to understand. It clearly signals that the > sequence number cycles through a range of 4096 values. > It also makes the code robust against potential changes of the 4096 > upper limit, especially when it becomes a non power-of-2 value while > the AND(&) works solely for power-of-2 values. > > The use of `% 4096u` also guarantees that the modulo operation is > performed with unsigned arithmetic, preventing potential issues with > the signed types. > > Found by Coccinelle. > > Suggested by Andy Shevchenko <[email protected]> > Signed-off-by: Abraham Samuel Adekunle <[email protected]> > --- > Coccinelle semantic patch used to find cases: > @@ > expression e; > > @@ > * e & 0xfff > > To ensure this change does not affect the functional > behaviour, I compared the generated object files before and > after the change using the `cmp` which compares the two > object files byte by byte as shown below: > > $ make drivers/staging/rtl8723bs/core/rtw_xmit.o > $ cmp rtw_xmit_before.o rtw_xmit_after.o > > No differences were found in the output, confirming that the > change does not alter the compiled output. This is version 11, right? What happened to the list of previous versions and what changed down here? confused, greg k-h