On 13/10/20 5:31 pm, Ujjwal Kumar wrote:
> checkpatch.pl 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 having
> execute permissions, without '.pl' extension in its filename
> and not belonging to 'scripts/' directory is reported as ERROR
> which is a false positive.
>
> Adding a shebang check along with current conditions will make
> the check more generalised and improve checkpatch reports.
> To do so, without breaking the core design decision of checkpatch,
> we can fetch the first line from the patch itself and match it for
> a shebang pattern.
>
> There can be cases where the first line is not part of the patch.
> For instance: a patch that only changes permissions without
> changing any of the file content.
> In that case there may be a false positive report but in the end we
> will have less false positives as we will be handling some of the
> unhandled cases.
>
> Signed-off-by: Ujjwal Kumar <ujjwalkumar0...@gmail.com>
> ---
> Changes in v2:
> - Spelling correction and add example to commit
> message
> - Code style changes
> - Remove unncessary function argument
> - Use non-capturing group in regexp
>
> scripts/checkpatch.pl | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fab38b493cef..7ebbee9c3672 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1795,6 +1795,23 @@ sub get_stat_here {
> return $herectx;
> }
>
> +sub get_shebang {
> + my ($linenr) = @_;
> + my $rawline = "";
> + my $shebang = "";
> +
> + $rawline = raw_line($linenr, 3);
I'm wondering if the range information can be at a
different offset from the 'new mode line'.
> + 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);
> if ($realfile !~ m@scripts/@ &&
> + $shebang !~ /^#!\s*(?:\/\w)+.*/ &&
> $realfile !~ /\.(py|pl|awk|sh)$/) {
Consider the following case:
a python script file with '.py' filename extension but without
a shebang line. Would it be meaningful to allow execute permission
on such a file?
> ERROR("EXECUTE_PERMISSIONS",
> "do not set execute permissions for
> source files\n" . $permhere);
>
> base-commit: 148fdf990dee4efd23c1114811b205de9c966680
> --
> 2.26.2
>
Thanks
Ujjwal Kumar
