Messages in this thread |  | | From | Sevinj Aghayeva <> | Date | Sun, 3 Apr 2022 12:12:43 -0400 | Subject | Re: [PATCH] staging: r8188eu: simplify control flow |
| |
Hi Pavel,
On Sun, Apr 3, 2022 at 11:59 AM Pavel Skripkin <paskripkin@gmail.com> wrote: > > Hi Sevinj, > > On 4/3/22 18:51, Sevinj Aghayeva wrote: > > The function iterates an index from 0 to NUM_PMKID_CACHE and returns > > the first index for which the condition is true. If no such index is > > found, the function returns -1. Current code has a complex control > > flow that obfuscates this simple task. Replace it with a loop. > > > > Also, given the shortened function body, replace the long variable > > name psecuritypriv with a short variable name p. > > > > Reported by checkpatch: > > > > WARNING: else is not generally useful after a break or return > > > > Signed-off-by: Sevinj Aghayeva <sevinj.aghayeva@gmail.com> > > --- > > [code snip] > > > + for (i = 0; i < NUM_PMKID_CACHE; i++) > > + if ((p->PMKIDList[i].bUsed) && > > + (!memcmp(p->PMKIDList[i].Bssid, bssid, ETH_ALEN))) > > + return i; > > + return -1; > > } > > > > /* */ > > Looks good, but let's not introduce new checkpatch issue: > > CHECK: Alignment should match open parenthesis > #62: FILE: drivers/staging/r8188eu/core/rtw_mlme.c:1645: > + if ((p->PMKIDList[i].bUsed) && > + (!memcmp(p->PMKIDList[i].Bssid, bssid, ETH_ALEN)))
Thanks for catching this. I wasn't seeing this in my checkpatch output, and after some digging, I could reproduce it with --strict option. I think the tutorial at https://kernelnewbies.org/PatchPhilosophy doesn't mention this option, so perhaps we should update it?!
> > > > > With regards, > Pavel Skripkin
--
Sevinj.Aghayeva
|  |