On Sun, 20 Oct 2019, Joe Perches wrote: > On Sun, 2019-10-20 at 22:17 +0300, Dan Carpenter wrote: > > On Sat, Oct 19, 2019 at 01:02:31PM -0700, Joe Perches wrote: > > > diff -u -p a/rtl8723bs/core/rtw_mlme_ext.c b/rtl8723bs/core/rtw_mlme_ext.c > [] > > > @@ -1132,7 +1132,7 @@ unsigned int OnAuthClient(struct adapter > > > goto authclnt_fail; > > > } > > > > > > - memcpy((void *)(pmlmeinfo->chg_txt), (void *)(p + 2), len); > > > + memcpy((void *)(pmlmeinfo->chg_txt), (p + 2), len); > > > > I wonder why it didn't remove the first void cast? > > drivers/staging/rtl8723bs/include/sta_info.h:151: unsigned char chg_txt[128]; > > I think the cocci transforms for an array do not match a pointer This is also correct. julia > and I wrote the cocci script without much care. > > btw; > > There's probably a generic cocci mechanism to check function > prototypes and then remove uses of unnecessary void pointer casts > in function calls. I'm not going to try to figure out that syntax. > > > [ The rest of the email is bonus comments for outreachy developers ]. > > > > And someone needs to check the final patch probably to remove the extra > > parentheses around "(p + 2)". Those were necessary when for the cast > > but not required after the cast is gone. > > > > > pmlmeinfo->auth_seq = 3; > > > issue_auth(padapter, NULL, 0); > > > set_link_timer(pmlmeext, REAUTH_TO); > > > > It's sort of tricky to know what "one thing per patch means". > > It seems somewhat arbitrary and based on Greg's understanding > of the experience of the patch submitter and also the language > of the potential commit message. > > > - memset((void *)(&(pHTInfo->SelfHTCap)), 0, > > + memset((&(pHTInfo->SelfHTCap)), 0, > > sizeof(pHTInfo->SelfHTCap)); > > > > Here the parentheses were never related to the cast so we should leave > > them as is. In other words, in the first example, if we didn't remove > > the cast that would be "half a thing per patch" and in the second > > example that would be "two things in one patch". > > For style patches, it's frequently easier and better to > do all the code transformation at once. > > IMO the last should be: > > memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap)); > > like it is here: > > drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c:1056: memset(&pHTInfo->SelfHTCap, 0, sizeof(pHTInfo->SelfHTCap)); > > btw2: > > I really dislike all the code inconsistencies and > unnecessary code duplication with miscellaneous changes > in the rtl staging drivers.... > > Horrid stuff. > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/6e6bc92cac0858fe5bd37b28f688d3da043f4bef.camel%40perches.com. > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
