Re: [PATCH 3/3] checkpatch: throw error in malformed arrays
From: Joe Perches
Date: Thu Feb 13 2025 - 06:53:46 EST
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.