linux-kernel - Re: [PATCH 3/3] checkpatch: throw error in malformed arrays

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3a6b43bad48592887d4e67ca854eef2728464b1c.camel@perches.com>
Date: Thu, 13 Feb 2025 03:17:09 -0800
From: Joe Perches <joe@...ches.com>
To: Guilherme Giacomo Simoes <trintaeoitogc@...il.com>,
 a.hindborg@...nel.org, 	alex.gaynor@...il.com, aliceryhl@...gle.com,
 apw@...onical.com, arnd@...db.de, 	aswinunni01@...il.com, axboe@...nel.dk,
 benno.lossin@...ton.me, 	bhelgaas@...gle.com, bjorn3_gh@...tonmail.com,
 boqun.feng@...il.com, 	dakr@...nel.org, dwaipayanray1@...il.com,
 ethan.twardy@...il.com, 	fujita.tomonori@...il.com, gary@...yguo.net,
 gregkh@...uxfoundation.org, 	lukas.bulwahn@...il.com, ojeda@...nel.org,
 pbonzini@...hat.com, tmgross@...ch.edu, 	walmeida@...rosoft.com
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] checkpatch: throw error in malformed arrays

On Wed, 2025-02-12 at 16:48 -0300, Guilherme Giacomo Simoes wrote:
> In module! macro, some fields have a string array type, and we need a
> check for guarantee a good formatation.

Isn't this is a style desire not a parsing limitation?

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2775,6 +2775,9 @@ sub process {
>  	$realcnt = 0;
>  	$linenr = 0;
>  	$fixlinenr = -1;
> +
> +	my %parse_module_arr;

Probably not useful to add hungarian style naming.

> @@ -3567,6 +3570,71 @@ sub process {
>  # ignore non-hunk lines and lines being removed
>  		next if (!$hunk_line || $line =~ /^-/);
>  
> +# check if arrays has more than one value in the same line
> +		my $inline = 0;
> +		if ($line =~ /^\+\s*.*\b(author|alias|firmware)\s*:\s*\[/) {

\s*.* could be just .*

> +			$inline = 1;
> +
> +			if ($line =~ /author/) {
> +				$parse_module_arr{'author'} = 1;
> +			}
> +
> +			if ($line =~ /alias/) {
> +				$parse_module_arr{'alias'} = 1;
> +			}
> +
> +			if ($line =~ /firmware/) {
> +				$parse_module_arr{'firmware'} = 1;
> +			}

$parse_module_arr{$1} = 1;

> +		}
> +
> +		if (keys %parse_module_arr) {
> +			my @keys = keys %parse_module_arr;
> +			my $key = $keys[0];
> +
> +			my $more_than_one_vl = qr/".+",\s*".+"/;
> +
> +			if ($line =~ $more_than_one_vl) {
> +					ERROR("ERR_ARRAY_MODULE_MACRO",
> +						"the key $key have more than one values in same line\n$here\n". cat_vet($rawline));

"Prefer one value per line\n"

ERR_ error prefixes are not useful.
These seem more like a WARN than an ERROR
Bad indentation
And please use a $herevet temporary


> +			}
> +			elsif ($inline && $line !~ /\]/ && $line =~ /\["/) {
> +					ERROR("ERR_ARRAY_MODULE_MACRO",
> +						qq~the key $key need a new line after declare the key\n$here\n~ . cat_vet($rawline) . qq~\nFor example:

Use } elsif

"Prefer declaring keys on separate lines"

Are quote characters allowed in keys?
If not, this seems unnecessarily complicated.

$herevet

I think the qq is unnecessary and the for example
should refer to some style documentation and not
be explicit in checkpatch.