Add 3 new warnings to checkpatch:
1) PREFER_ETHER_ADDR_EQUAL
Replace memcmp(foo, bar, ETH_ALEN) with ether_addr_equal*()
2) PREFER_ETH_ZERO_ADDR
Replace memset(foo, 0, ETH_ALEN) with eth_zero_addr()
3) PREFER_ETH_BROADCAST_ADDR
Replace memset(foo, 0xFF, ETH_ALEN) with eth_broadcast_addr()
Mateusz Kulikowski (2):
checkpatch: suggest using ether_addr_equal*()
checkpatch: suggest using eth_zero_addr() and eth_broadcast_addr()
scripts/checkpatch.pl | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
--
1.8.4.1
Check if memcmp() is used to compare ethernet addresses and suggest using
ether_addr_equal() or ether_addr_equal_unaligned()
Signed-off-by: Mateusz Kulikowski <[email protected]>
---
scripts/checkpatch.pl | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 89b1df4..3a1a01e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5035,6 +5035,13 @@ sub process {
}
}
+# Check for memcmp(foo, bar, ETH_ALEN) that could be ether_addr_equal*(foo, bar)
+ if ($^V && $^V ge 5.10.0 &&
+ $line =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) {
+ WARN("PREFER_ETHER_ADDR_EQUAL",
+ "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . $herecurr)
+ }
+
# typecasts on min/max could be min_t/max_t
if ($^V && $^V ge 5.10.0 &&
defined $stat &&
--
1.8.4.1
Suggest using eth_zero_addr() or eth_broadcast_addr() if memset is used instead.
Signed-off-by: Mateusz Kulikowski <[email protected]>
---
scripts/checkpatch.pl | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3a1a01e..1fc0819 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5042,6 +5042,22 @@ sub process {
"Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . $herecurr)
}
+# check for memset(foo, 0x0, ETH_ALEN) that could be eth_zero_addr
+# check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr
+ if ($^V && $^V ge 5.10.0 &&
+ $line =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) {
+
+ my $ms_val = $7;
+
+ if ($ms_val =~ /^(0x|)0$/i) {
+ WARN("PREFER_ETH_ZERO_ADDR",
+ "Prefer eth_zero_addr over memset()\n" . $herecurr);
+ } elsif ($ms_val =~ /^0xff$/i) {
+ WARN("PREFER_ETH_BROADCAST_ADDR",
+ "Prefer eth_broadcast_addr() over memset()\n" . $herecurr);
+ }
+ }
+
# typecasts on min/max could be min_t/max_t
if ($^V && $^V ge 5.10.0 &&
defined $stat &&
--
1.8.4.1
On Sun, 2015-04-19 at 21:04 +0200, Mateusz Kulikowski wrote:
> Suggest using eth_zero_addr() or eth_broadcast_addr() if memset is used instead.
Hi Mateusz, this is OK but here are some notes:
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5042,6 +5042,22 @@ sub process {
[]
> +# check for memset(foo, 0x0, ETH_ALEN) that could be eth_zero_addr
> +# check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr
> + if ($^V && $^V ge 5.10.0 &&
> + $line =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) {
Please align the $line with the $^V above it.
> +
> + my $ms_val = $7;
> +
> + if ($ms_val =~ /^(0x|)0$/i) {
I've seen 0x00 many times so: /^(?:0x|)0+$/
> + WARN("PREFER_ETH_ZERO_ADDR",
> + "Prefer eth_zero_addr over memset()\n" . $herecurr);
Please align to the initial open quote here:
WARN("PREFER_ETH_ZERO_ADDR",
"Prefer eth_zero_addr over memset()\n" . $herecurr);
> + } elsif ($ms_val =~ /^0xff$/i) {
255 here too so: /^(?:0xff|255)$/
> + WARN("PREFER_ETH_BROADCAST_ADDR",
> + "Prefer eth_broadcast_addr() over memset()\n" . $herecurr);
Please align to open quote
On Sun, 2015-04-19 at 21:04 +0200, Mateusz Kulikowski wrote:
> Check if memcmp() is used to compare ethernet addresses and suggest using
> ether_addr_equal() or ether_addr_equal_unaligned()
Hi again Mateusz.
This is OK with me.
If you ever submit that ether_addr_copy_unaligned patch,
please update the checkpatch test for ether_addr_copy too.
Joe
Hi Joe,
On 19.04.2015 22:20, Joe Perches wrote:
(...)
>> +# check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr
>> + if ($^V && $^V ge 5.10.0 &&
>> + $line =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) {
>
> Please align the $line with the $^V above it.
Thanks for review - I'll post v2 soon;
Sorry for alignment issues - shame on me and my editor.
(I've found that PATCH 1/2 was also not aligned - will fix it as well)
Regards,
Mateusz