2015-02-25 02:40:51

by Aya Mahfouz

[permalink] [raw]
Subject: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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 <[email protected]>
Signed-off-by: Aya Mahfouz <[email protected]>
---
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


2015-02-25 03:10:57

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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)/;
> + }
> }
> }
>


2015-02-25 04:35:30

by Aya Mahfouz

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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");
}
}

2015-02-25 04:41:27

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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.

2015-02-25 04:59:14

by Aya Mahfouz

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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

2015-02-25 05:45:47

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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)/;
}
}


2015-02-25 06:08:42

by Aya Mahfouz

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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

2015-02-25 06:20:30

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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

2015-02-27 01:42:51

by Aya Mahfouz

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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

2015-02-27 01:57:29

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2] scripts: checkpatch.pl: add 2 new checks on memset calls

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