Re: [PATCH] checkpatch: Check for .byte-spelled insn opcodes documentation on x86

[Joe Perches]

On Fri, 2020-10-09 at 18:14 +0200, Borislav Petkov wrote:
> From: Borislav Petkov <bp@xxxxxxx>
> 
> Instruction opcode bytes spelled using the gas directive .byte should
> carry a comment above them stating which binutils version has added
> support for the instruction mnemonic so that they can be replaced with
> the mnemonic when that binutils version is equal or less than the
> minimum-supported version by the kernel.
> 
> Add a check for that.

OK but several notes:

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -6858,6 +6858,18 @@ sub process {
>  			WARN("DUPLICATED_SYSCTL_CONST",
>  				"duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr);
>  		}
> +
> +# document which binutils version supports the actual insn mnemonic so that the naked opcode bytes can be replaced.
> +# x86-only.
> +		if ($rawline =~ /(\.byte(?:0[xX][0-9a-fA-F]+0-9a-fx,]+)/ && $realfile =~ "^arch/x86/") {

Given the location, this only works on .c and .h files.
It does not work on .S files.  Should it?

No need for a capture group.

Please use @ not " as all the other $realfile comparisons
use that form when expecting a /

So it looks like the regex would be more complete as:

	if ($realfile =~ m@^arch/x86/@ &&
	    $rawline =~ /\.byte\s+(?:$Constant|(?:\\)?$Ident|"\s*$Ident)\b/) {

etc...

> +			my $comment = ctx_locate_comment(0, $linenr);

A patch can modify any number of files.

This should use ctx_locate_comment($file ? 0 : $first_line, $linenr)
as checkpatch tests work on patch contexts not the entire
file before this line.

> +			if (! $comment || ($comment !~ /binutils version [0-9.]+/(ms)) {

No need for the $!comment test

> +				WARN("MISSING_BINUTILS_VERSION",
> +				     "Please document which binutils version supports these .byte-spelled\n" .
> +				     "\tinsn opcodes by adding \"binutils version <num>\" in a comment" .
> +				     " above them.\n" . $herecurr);

checkpatch uses only a single line output only before $herecurr
Output line length doesn't matter.