* [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls @ 2015-02-25 2:40 Aya Mahfouz 2015-02-25 3:10 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Aya Mahfouz @ 2015-02-25 2:40 UTC (permalink / raw) To: Andy Whitcroft, Joe Perches, linux-kernel This patch adds 2 new checks on memset calls in the file checkpatch.pl as follows: replace memset by eth_zero_addr if the second argument is an address of zeros (0x00). eth_zero_addr is a wrapper function for memset that takes an address array to set as zero. The size address has to be ETH_ALEN. replace memset by eth_broadcast_addr if the second argument is the broadcast address (0xff). eth_broadcast_addr is a wrapper function for memset that sets the passed array the broadcast address. The size of the address has to be ETH_ALEN. Cc: Julia Lawall <julia.lawall@lip6.fr> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com> --- v2: adjusted all checks on memset calls to be in one body scripts/checkpatch.pl | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d124359..ab43d1b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4890,10 +4890,10 @@ sub process { } } -# Check for misused memsets +# Check for misused memsets then check for memset(foo, 0x00|0xff, ETH_ALEN) +# calls that could be eth_zero_addr(foo)|eth_broadcast_addr(foo) if ($^V && $^V ge 5.10.0 && - defined $stat && - $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { + $line =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { my $ms_addr = $2; my $ms_val = $7; @@ -4901,10 +4901,22 @@ sub process { if ($ms_size =~ /^(0x|)0$/i) { ERROR("MEMSET", - "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n"); + "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$line\n"); } elsif ($ms_size =~ /^(0x|)1$/i) { WARN("MEMSET", - "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n"); + "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$line\n"); + } elsif ($ms_size =~ /^ETH_ALEN/i) { + if ($ms_val =~ /^0x00/i && WARN("PREFER_ETH_ZERO_ADDR", + "Prefer eth_zero_addr() over memset() if the second address is 0x00\n" . $herecurr) && + $fix) { + + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/eth_zero_addr($ms_addr)/; + } elsif ($ms_val =~ /^0xff/i && WARN("PREFER_ETH_BROADCAST_ADDR", + "Prefer eth_broadcast_addr() over memset() if the second address is 0xff\n" . $herecurr) && + $fix) { + + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/eth_broadcast_addr($ms_addr)/; + } } } -- 1.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-25 2:40 [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls Aya Mahfouz @ 2015-02-25 3:10 ` Joe Perches 2015-02-25 4:35 ` Aya Mahfouz 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2015-02-25 3:10 UTC (permalink / raw) To: Aya Mahfouz; +Cc: Andy Whitcroft, linux-kernel On Wed, 2015-02-25 at 04:40 +0200, Aya Mahfouz wrote: > This patch adds 2 new checks on memset calls in the file > checkpatch.pl as follows: > > replace memset by eth_zero_addr if the second argument is > an address of zeros (0x00). eth_zero_addr is a wrapper function > for memset that takes an address array to set as zero. The size > address has to be ETH_ALEN. [] > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -4901,10 +4901,22 @@ sub process { > > if ($ms_size =~ /^(0x|)0$/i) { > ERROR("MEMSET", > - "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n"); > + "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$line\n"); > } elsif ($ms_size =~ /^(0x|)1$/i) { > WARN("MEMSET", > - "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n"); > + "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$line\n"); > + } elsif ($ms_size =~ /^ETH_ALEN/i) { > + if ($ms_val =~ /^0x00/i && WARN("PREFER_ETH_ZERO_ADDR", This isn't right. Look again at what I suggested. This would match 0x00ff and wouldn't match 0 > + "Prefer eth_zero_addr() over memset() if the second address is 0x00\n" . $herecurr) && > + $fix) { > + > + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/eth_zero_addr($ms_addr)/; > + } elsif ($ms_val =~ /^0xff/i && WARN("PREFER_ETH_BROADCAST_ADDR", > + "Prefer eth_broadcast_addr() over memset() if the second address is 0xff\n" . $herecurr) && > + $fix) { > + > + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/eth_broadcast_addr($ms_addr)/; > + } > } > } > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-25 3:10 ` Joe Perches @ 2015-02-25 4:35 ` Aya Mahfouz 2015-02-25 4:41 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Aya Mahfouz @ 2015-02-25 4:35 UTC (permalink / raw) To: Joe Perches; +Cc: Andy Whitcroft, linux-kernel On Tue, Feb 24, 2015 at 07:10:52PM -0800, Joe Perches wrote: > On Wed, 2015-02-25 at 04:40 +0200, Aya Mahfouz wrote: > > This patch adds 2 new checks on memset calls in the file > > checkpatch.pl as follows: > > > > replace memset by eth_zero_addr if the second argument is > > an address of zeros (0x00). eth_zero_addr is a wrapper function > > for memset that takes an address array to set as zero. The size > > address has to be ETH_ALEN. > > [] > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] > > @@ -4901,10 +4901,22 @@ sub process { > > > > if ($ms_size =~ /^(0x|)0$/i) { > > ERROR("MEMSET", > > - "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n"); > > + "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$line\n"); > > } elsif ($ms_size =~ /^(0x|)1$/i) { > > WARN("MEMSET", > > - "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n"); > > + "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$line\n"); > > + } elsif ($ms_size =~ /^ETH_ALEN/i) { > > + if ($ms_val =~ /^0x00/i && WARN("PREFER_ETH_ZERO_ADDR", > > This isn't right. Look again at what I suggested. > > This would match 0x00ff and wouldn't match 0 > > > + "Prefer eth_zero_addr() over memset() if the second address is 0x00\n" . $herecurr) && > > + $fix) { > > + > > + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/eth_zero_addr($ms_addr)/; > > + } elsif ($ms_val =~ /^0xff/i && WARN("PREFER_ETH_BROADCAST_ADDR", > > + "Prefer eth_broadcast_addr() over memset() if the second address is 0xff\n" . $herecurr) && > > + $fix) { > > + > > + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/eth_broadcast_addr($ms_addr)/; > > + } > > } > > } > > > > > ok, I didn't see your suggestion, sorry. Can you look at the following modification before sending the third patch? I don't use $stat because I get false positives with it. #check for misused memsets if ($^V && $^V ge 5.10.0 && $line =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { my $ms_addr = $2; my $ms_val = $7; my $ms_size = $12; if ($ms_val =~ /^(?:0|0x0+)$/i && $ms_size =~ /^ETH_ALEN$/ && WARN("PREFER_ETH_ADDR_FUNC", "Prefer eth_zero_addr() over memset() if the second address is 0x00\n" . $herecurr) && $fix) { $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*\Q$ms_addr\E\s*,\s*\Q$ms_val\E\s*\,\s*ETH_ALEN\s*\)/eth_zero_addr($ms_addr)/; } elsif ($ms_val =~ /^(?:0xff|255)$/i && $ms_size =~ /^ETH_ALEN$/ && WARN("PREFER_ETH_ADDR_FUNC", "Prefer eth_broadcast_addr() over memset() if the second address is 0xff\n" . $herecurr) && $fix) { $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*\Q$ms_addr\E\s*,\s*\Q$ms_val\E\s*\,\s*ETH_ALEN\s*\)/eth_broadcast_addr($ms_addr)/; } elsif ($ms_size =~ /^(0x|)0$/i) { ERROR("MEMSET", "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$line\n"); } elsif ($ms_size =~ /^(0x|)1$/i) { WARN("MEMSET", "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$line\n"); } } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-25 4:35 ` Aya Mahfouz @ 2015-02-25 4:41 ` Joe Perches 2015-02-25 4:59 ` Aya Mahfouz 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2015-02-25 4:41 UTC (permalink / raw) To: Aya Mahfouz; +Cc: Andy Whitcroft, linux-kernel On Wed, 2015-02-25 at 06:35 +0200, Aya Mahfouz wrote: > On Tue, Feb 24, 2015 at 07:10:52PM -0800, Joe Perches wrote: > > On Wed, 2015-02-25 at 04:40 +0200, Aya Mahfouz wrote: > > > This patch adds 2 new checks on memset calls in the file > > > checkpatch.pl as follows: [] > ok, I didn't see your suggestion, sorry. No worries. > Can you look at the following > modification before sending the third patch? I don't use $stat because > I get false positives with it. Please describe the false positives. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-25 4:41 ` Joe Perches @ 2015-02-25 4:59 ` Aya Mahfouz 2015-02-25 5:45 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Aya Mahfouz @ 2015-02-25 4:59 UTC (permalink / raw) To: Joe Perches; +Cc: Andy Whitcroft, linux-kernel On Tue, Feb 24, 2015 at 08:41:23PM -0800, Joe Perches wrote: > On Wed, 2015-02-25 at 06:35 +0200, Aya Mahfouz wrote: > > On Tue, Feb 24, 2015 at 07:10:52PM -0800, Joe Perches wrote: > > > On Wed, 2015-02-25 at 04:40 +0200, Aya Mahfouz wrote: > > > > This patch adds 2 new checks on memset calls in the file > > > > checkpatch.pl as follows: > [] > > ok, I didn't see your suggestion, sorry. > > No worries. > > > Can you look at the following > > modification before sending the third patch? I don't use $stat because > > I get false positives with it. > > Please describe the false positives. > > ok, here are the relevant warnings issued by checkpatch.pl when using $stat for the file drivers/staging/rtl8188eu/os_dep/ioctl_linux.c. The only correct results are lines 95, 830, 1031, 1040, 1908. WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #95: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:95: + memset(wrqu.ap_addr.sa_data, 0, ETH_ALEN); WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #775: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:775: +} WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #777: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:777: +static int rtw_wx_set_pmkid(struct net_device *dev, WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #778: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:778: + struct iw_request_info *a, WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #779: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:779: + union iwreq_data *wrqu, char *extra) WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #780: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:780: +{ WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #823: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:823: + } WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #824: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:824: + } else if (pPMK->cmd == IW_PMKSA_REMOVE) { WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #827: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:827: + for (j = 0; j < NUM_PMKID_CACHE; j++) { WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #828: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:828: + if (!memcmp(psecuritypriv->PMKIDList[j].Bssid, strIssueBssid, ETH_ALEN)) { WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #830: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:830: + memset(psecuritypriv->PMKIDList[j].Bssid, 0x00, ETH_ALEN); WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #1019: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1019: +} WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #1021: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1021: +static int rtw_wx_get_wap(struct net_device *dev, WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #1022: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1022: + struct iw_request_info *info, WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #1023: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1023: + union iwreq_data *wrqu, char *extra) WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #1024: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1024: +{ WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #1031: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1031: + memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN); WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #1039: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1039: + else WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 #1040: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1040: + memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN); WARNING: Prefer eth_broadcast_addr() over memset() if the second address is 0xff #1908: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:1908: + memset(param->sta_addr, 0xff, ETH_ALEN); -- Kind Regards, Aya Saif El-yazal Mahfouz ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-25 4:59 ` Aya Mahfouz @ 2015-02-25 5:45 ` Joe Perches 2015-02-25 6:08 ` Aya Mahfouz 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2015-02-25 5:45 UTC (permalink / raw) To: Aya Mahfouz; +Cc: Andy Whitcroft, linux-kernel On Wed, 2015-02-25 at 06:59 +0200, Aya Mahfouz wrote: > On Tue, Feb 24, 2015 at 08:41:23PM -0800, Joe Perches wrote: > > On Wed, 2015-02-25 at 06:35 +0200, Aya Mahfouz wrote: > > > On Tue, Feb 24, 2015 at 07:10:52PM -0800, Joe Perches wrote: > > > > On Wed, 2015-02-25 at 04:40 +0200, Aya Mahfouz wrote: > > > > > This patch adds 2 new checks on memset calls in the file > > > > > checkpatch.pl as follows: > > [] > > > ok, I didn't see your suggestion, sorry. > > > > No worries. > > > > > Can you look at the following > > > modification before sending the third patch? I don't use $stat because > > > I get false positives with it. > > > > Please describe the false positives. > > > > > > ok, here are the relevant warnings issued by checkpatch.pl when using > $stat for the file drivers/staging/rtl8188eu/os_dep/ioctl_linux.c. > The only correct results are lines 95, 830, 1031, 1040, 1908. > > WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 > #95: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:95: > + memset(wrqu.ap_addr.sa_data, 0, ETH_ALEN); > > > WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 > #775: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:775: > +} [] Try this: --- scripts/checkpatch.pl | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d124359..9127c65 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4890,10 +4890,11 @@ sub process { } } -# Check for misused memsets +# Check for misused memsets then check for memset(foo, 0x00|0xff, ETH_ALEN) +# calls that could be eth_zero_addr(foo)|eth_broadcast_addr(foo) if ($^V && $^V ge 5.10.0 && defined $stat && - $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { + $stat =~ /^\+(?:\s*$Ident\s*=)?\s*memset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { my $ms_addr = $2; my $ms_val = $7; @@ -4901,10 +4902,22 @@ sub process { if ($ms_size =~ /^(0x|)0$/i) { ERROR("MEMSET", - "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n"); + "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$line\n"); } elsif ($ms_size =~ /^(0x|)1$/i) { WARN("MEMSET", - "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n"); + "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$line\n"); + } elsif ($ms_val =~ /^(?:0x)?0+$/i && + $ms_size =~ /^ETH_ALEN$/ && + WARN("PREFER_ETH_ADDR", + "Prefer eth_zero_addr() over memset() if the second address is 0\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*\Q$ms_addr\E\s*,\s*\Q$ms_val\E\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($ms_addr)/; + } elsif ($ms_val =~ /^(?:0xff|255)$/i && + $ms_size =~ /^ETH_ALEN$/ && + WARN("PREFER_ETH_ADDR", + "Prefer eth_broadcast_addr() over memset() if the second address is 0xff\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*\Q$ms_addr\E\s*,\s*\Q$ms_val\E\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($ms_addr)/; } } ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-25 5:45 ` Joe Perches @ 2015-02-25 6:08 ` Aya Mahfouz 2015-02-25 6:20 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Aya Mahfouz @ 2015-02-25 6:08 UTC (permalink / raw) To: Joe Perches; +Cc: Andy Whitcroft, linux-kernel On Tue, Feb 24, 2015 at 09:45:43PM -0800, Joe Perches wrote: > On Wed, 2015-02-25 at 06:59 +0200, Aya Mahfouz wrote: > > On Tue, Feb 24, 2015 at 08:41:23PM -0800, Joe Perches wrote: > > > On Wed, 2015-02-25 at 06:35 +0200, Aya Mahfouz wrote: > > > > On Tue, Feb 24, 2015 at 07:10:52PM -0800, Joe Perches wrote: > > > > > On Wed, 2015-02-25 at 04:40 +0200, Aya Mahfouz wrote: > > > > > > This patch adds 2 new checks on memset calls in the file > > > > > > checkpatch.pl as follows: > > > [] > > > > ok, I didn't see your suggestion, sorry. > > > > > > No worries. > > > > > > > Can you look at the following > > > > modification before sending the third patch? I don't use $stat because > > > > I get false positives with it. > > > > > > Please describe the false positives. > > > > > > > > > > ok, here are the relevant warnings issued by checkpatch.pl when using > > $stat for the file drivers/staging/rtl8188eu/os_dep/ioctl_linux.c. > > The only correct results are lines 95, 830, 1031, 1040, 1908. > > > > WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 > > #95: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:95: > > + memset(wrqu.ap_addr.sa_data, 0, ETH_ALEN); > > > > > > WARNING: Prefer eth_zero_addr() over memset() if the second address is 0x00 > > #775: FILE: drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:775: > > +} > > [] > > Try this: > --- > scripts/checkpatch.pl | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index d124359..9127c65 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -4890,10 +4890,11 @@ sub process { > } > } > > -# Check for misused memsets > +# Check for misused memsets then check for memset(foo, 0x00|0xff, ETH_ALEN) > +# calls that could be eth_zero_addr(foo)|eth_broadcast_addr(foo) > if ($^V && $^V ge 5.10.0 && > defined $stat && > - $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { > + $stat =~ /^\+(?:\s*$Ident\s*=)?\s*memset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { > > my $ms_addr = $2; > my $ms_val = $7; > @@ -4901,10 +4902,22 @@ sub process { > > if ($ms_size =~ /^(0x|)0$/i) { > ERROR("MEMSET", > - "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$stat\n"); > + "memset to 0's uses 0 as the 2nd argument, not the 3rd\n" . "$here\n$line\n"); > } elsif ($ms_size =~ /^(0x|)1$/i) { > WARN("MEMSET", > - "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$stat\n"); > + "single byte memset is suspicious. Swapped 2nd/3rd argument?\n" . "$here\n$line\n"); > + } elsif ($ms_val =~ /^(?:0x)?0+$/i && > + $ms_size =~ /^ETH_ALEN$/ && > + WARN("PREFER_ETH_ADDR", > + "Prefer eth_zero_addr() over memset() if the second address is 0\n" . $herecurr) && > + $fix) { > + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*\Q$ms_addr\E\s*,\s*\Q$ms_val\E\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($ms_addr)/; > + } elsif ($ms_val =~ /^(?:0xff|255)$/i && > + $ms_size =~ /^ETH_ALEN$/ && > + WARN("PREFER_ETH_ADDR", > + "Prefer eth_broadcast_addr() over memset() if the second address is 0xff\n" . $herecurr) && > + $fix) { > + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*\Q$ms_addr\E\s*,\s*\Q$ms_val\E\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($ms_addr)/; > } > } > > > Yes, this patch works smoothly. I'm not getting the false positives now. What is the next step? -- Kind Regards, Aya Saif El-yazal Mahfouz ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-25 6:08 ` Aya Mahfouz @ 2015-02-25 6:20 ` Joe Perches 2015-02-27 1:42 ` Aya Mahfouz 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2015-02-25 6:20 UTC (permalink / raw) To: Aya Mahfouz; +Cc: Andy Whitcroft, linux-kernel On Wed, 2015-02-25 at 08:08 +0200, Aya Mahfouz wrote: > What is the next step? Ideally, you understanding why the suggested patch is an improvement over the patch you proposed. Beyond that, submit a patch. Then submit more... cheers, Joe ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-25 6:20 ` Joe Perches @ 2015-02-27 1:42 ` Aya Mahfouz 2015-02-27 1:57 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Aya Mahfouz @ 2015-02-27 1:42 UTC (permalink / raw) To: Joe Perches; +Cc: julia.lawall, Andy Whitcroft, linux-kernel On Tue, Feb 24, 2015 at 10:20:24PM -0800, Joe Perches wrote: > On Wed, 2015-02-25 at 08:08 +0200, Aya Mahfouz wrote: > > What is the next step? > > Ideally, you understanding why the suggested patch > is an improvement over the patch you proposed. > > Beyond that, submit a patch. Then submit more... > > cheers, Joe > Hello Joe, This is just a note that I did not forget about the patch. I understand how it works now. I'm just finalizing some final steps with Julia which might take some time. Thanks for all your help, -- Kind Regards, Aya Saif El-yazal Mahfouz ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls 2015-02-27 1:42 ` Aya Mahfouz @ 2015-02-27 1:57 ` Joe Perches 0 siblings, 0 replies; 10+ messages in thread From: Joe Perches @ 2015-02-27 1:57 UTC (permalink / raw) To: Aya Mahfouz; +Cc: julia.lawall, Andy Whitcroft, linux-kernel On Fri, 2015-02-27 at 03:42 +0200, Aya Mahfouz wrote: > On Tue, Feb 24, 2015 at 10:20:24PM -0800, Joe Perches wrote: > > On Wed, 2015-02-25 at 08:08 +0200, Aya Mahfouz wrote: > > > What is the next step? > > Ideally, you understanding why the suggested patch > > is an improvement over the patch you proposed. > > Beyond that, submit a patch. Then submit more... [] > This is just a note that I did not forget about the patch. Hello Aya > I understand how it works now. Great. > I'm just finalizing some final steps with Julia which > might take some time. No rush. Understanding is more important than the patch. > Thanks for all your help, You're quite welcome. No charge... Joe ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-02-27 1:57 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-25 2:40 [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls Aya Mahfouz 2015-02-25 3:10 ` Joe Perches 2015-02-25 4:35 ` Aya Mahfouz 2015-02-25 4:41 ` Joe Perches 2015-02-25 4:59 ` Aya Mahfouz 2015-02-25 5:45 ` Joe Perches 2015-02-25 6:08 ` Aya Mahfouz 2015-02-25 6:20 ` Joe Perches 2015-02-27 1:42 ` Aya Mahfouz 2015-02-27 1:57 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox