scripts: checkpatch.pl: add 2 new checks on memset calls

PATCH Superseded Aya Mahfouz 2 participants 3 messages Feb 23, 2015 – Feb 25, 2015 Checkpatch

lore

Share & tools

lore

Aya Mahfouz

author

2015-02-23 19:19 UTC #0 lore raw

PATCH 1/1 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]>

---
 scripts/checkpatch.pl | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

scripts/checkpatch.pl +19 -0

index d124359..9619882 100755
@@ -4918,6 +4918,25 @@ sub process {
 			}
 		}
 
+# Check for memset(foo, 0x00|0xff, ETH_ALEN) that could be eth_zero_addr(foo)|eth_broadcast_addr(foo)
+		if ($^V && $^V ge 5.10.0 &&
+		    $line =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) {
+			my $num = $7;
+			if ($num =~ "0x00" && 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($2)/;
+			}
+			elsif ($num =~ "0xff" && 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($2)/;
+			}
+		}
+
+
 # typecasts on min/max could be min_t/max_t
 		if ($^V && $^V ge 5.10.0 &&
 		    defined $stat &&

Signature · 1 line

1.9.3

Joe Perches

maintainer

2015-02-24 03:06 UTC #1 lore raw

Quoted 8 lines · Aya Mahfouz

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.

Quoted 4 lines

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.

Hello Aya.

Good idea but:

there already is a test for memset at (-next) line 4893.

Please extend that as below instead of adding another test.

scripts/checkpatch.pl

Quoted 1 line

scripts/checkpatch.pl +0 -0

[]

scripts/checkpatch.pl sub process { · line 4925

Quoted 9 lines

scripts/checkpatch.pl +5 -0

@@ -4918,6 +4918,25 @@ sub process {
 			}
 		}
 
+# Check for memset(foo, 0x00|0xff, ETH_ALEN) that could be eth_zero_addr(foo)|eth_broadcast_addr(foo)
+		if ($^V && $^V ge 5.10.0 &&
+		    $line =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) {
+			my $num = $7;
+			if ($num =~ "0x00" && WARN("PREFER_ETH_ZERO_ADDR",

# Check for misused memsets

		if ($^V && $^V ge 5.10.0 &&
		    defined $stat &&
		    $stat =~ /^\+(?:.*?)\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",
				 "..." . 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",
				      "..." . 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 (...)

Aya Mahfouz

author

2015-02-25 02:53 UTC #2 lore raw

Quoted 14 lines · Joe Perches

Quoted 8 lines · Aya Mahfouz
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.
Quoted 4 lines
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.

Quoted 8 lines

Hello Aya.
Good idea but:
there already is a test for memset at (-next) line 4893.
Please extend that as below instead of adding another test.

Hello Joe,

I've adjusted the checks as you requested. Kindly let me know if there is something else that should be done.

Kind Regards,
Aya Saif El-yazal Mahfouz

Quoted 36 lines