* [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