[Linux-kernel-mentees] [RFC PATCH] checkpatch: add shebang check to EXECUTE_PERMISSIONS

[Lukas Bulwahn]

On Sun, 11 Oct 2020, Ujjwal Kumar wrote:

> checkpatch script checks for invalid EXECUTE_PERMISSIONS on source
> files. The script leverages filename extensions and its path in
> the repository to decide whether to allow execute permissions on
> the file or not.
> 
> Based on current check conditions, a perl script file without
> '.pl' extension in its filename and not belonging to 'scripts/'
> directory is reported as ERROR which is a false-positive.
> 
> The script can correctly handle patches with mode changes and
> shebang line if shebang is taken into account. So, along with
> the current check conditions, adding the shebang check in the
> check conditions can improve the reports of the script.
>

I think one of the core design decisions of checkpatch.pl is:

checkpatch.pl can run on a patch, even if the patch does not apply to the 
current repository version that is checked out.

It solely uses the information in the patch, and does not try to guess how 
it could be applied etc.

This patch violates that core design decisions.

You can propose to Joe Perches and lkml, but do not be surprised if that 
is rejected because of this reason above.

I would be interested in the discussion.

Lukas

 
> Signed-off-by: Ujjwal Kumar <ujjwalkumar0501 at gmail.com>
> ---
>  scripts/checkpatch.pl | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fab38b493cef..e596d30794bf 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1795,6 +1795,23 @@ sub get_stat_here {
>  	return $herectx;
>  }
>  
> +sub get_shebang {
> +	my ($linenr, $realfile) = @_;
> +	my $rawline = "";
> +	my $shebang = "";
> +
> +	$rawline = raw_line($linenr, 3);
> +	if (defined $rawline &&
> +		$rawline =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
> +		if (defined $1 && $1 == 1) {
> +			$shebang = raw_line($linenr, 4);
> +			$shebang = substr $shebang, 1;
> +		}
> +	}
> +
> +	return $shebang;
> +}
> +
>  sub cat_vet {
>  	my ($vet) = @_;
>  	my ($res, $coded);
> @@ -2680,7 +2697,9 @@ sub process {
>  # Check for incorrect file permissions
>  		if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
>  			my $permhere = $here . "FILE: $realfile\n";
> +			my $shebang = get_shebang($linenr, $realfile);
>  			if ($realfile !~ m at scripts/@ &&
> +			    $shebang !~ /^#!\s*(\/\w)+.*/ &&
>  			    $realfile !~ /\.(py|pl|awk|sh)$/) {
>  				ERROR("EXECUTE_PERMISSIONS",
>  				      "do not set execute permissions for source files\n" . $permhere);
> 
> base-commit: d67bc7812221606e1886620a357b13f906814af7
> -- 
> 2.26.2
> 
>