[PATCH 0/2] checkpatch: new ethernet address manipulation checks

[PATCH 0/2] checkpatch: new ethernet address manipulation checks

[Mateusz Kulikowski]

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

[PATCH 1/2] checkpatch: suggest using ether_addr_equal*()

[Mateusz Kulikowski]

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 <mateusz.kulikowski@gmail.com>
---
 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

[PATCH 2/2] checkpatch: suggest using eth_zero_addr() and eth_broadcast_addr()

[Mateusz Kulikowski]

Suggest using eth_zero_addr() or eth_broadcast_addr() if memset is used instead.

Signed-off-by: Mateusz Kulikowski <mateusz.kulikowski@gmail.com>
---
 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

Re: [PATCH 2/2] checkpatch: suggest using eth_zero_addr() and eth_broadcast_addr()

[Joe Perches]

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

Re: [PATCH 1/2] checkpatch: suggest using ether_addr_equal*()

[Joe Perches]

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

Re: [PATCH 2/2] checkpatch: suggest using eth_zero_addr() and eth_broadcast_addr()

[Mateusz Kulikowski]

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