Re: [RFC PATCH] checkpatch: check for trivial sizeofs

From: Joe Perches
Date: Sat May 30 2020 - 19:10:52 EST


On Sun, 2020-05-31 at 00:21 +0300, Denis Efremov wrote:
> sizeof(char) and its variations in most cases doesn't make code more clear.
> It only makes code wordy.

There are about 1000 of these uses in the kernel.

Not sure I like this though as many/most of the uses
seem _less_ readable with a 1 in their place.

$ git grep -P 'sizeof\s*\(\s*(?:(?:unsigned\s+)?char\s*|(?:__)?u8|u?int8_t|[us]?byte(?:_t)?)\s*\)' | wc -l
970

Try the grep without the wc and see if you agree.

And if this is actually going to be used, I'd prefer
using a separate $typeChar for the search.

Maybe something like:
---
scripts/checkpatch.pl | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index dd750241958b..de2e9242350b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -466,6 +466,13 @@ our $typeTypedefs = qr{(?x:
$typeKernelTypedefs\b
)};

+our $typeChar = qr{(?x:
+ (?:(?:un)?signed\s+)?char |
+ (?:__)?u8 |
+ (?:u_|u)?int8_t |
+ [us]?byte(?:_t)?
+)};
+
our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b};

our $logFunctions = qr{(?x:
@@ -6106,6 +6113,16 @@ sub process {
}
}

+# check for trivial sizeof(char) == 1
+ if ($line =~ /\bsizeof\s*\(\s*($typeChar)\s*\)/) {
+ my $byte = $1;
+ if (CHK("SIZEOF_CHAR",
+ "sizeof($byte) could be the constant 1 instead\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/sizeof\s*\(\s*\Q$byte\E\s*\)/1/;
+ }
+ }
+
# check for struct spinlock declarations
if ($line =~ /^.\s*\bstruct\s+spinlock\s+\w+\s*;/) {
WARN("USE_SPINLOCK_T",