lkml.org 
[lkml]   [2020]   [Oct]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RFC PATCH v2] checkpatch: add shebang check to EXECUTE_PERMISSIONS


On Tue, 13 Oct 2020, 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.
>

I get the intent of your addition. However:

I would bet that you only find one or two in a million commits, that would
actually benefit for this special check of a special case of a special
rule...

So given the added complexity of yet another 19 lines in checkpatch with
little benefit of lowering false positive reports, I would be against
inclusion.

You can provide convincing arguments with an evaluation, where you show
on how many commits this change would really make a difference...

It is probably better and simpler to just have a script checking for
execute bits on all files in the repository on linux-next (with a list of
known intended executable files) and just report to you and then to the
developers when a new file with unintentional execute bit appeared.

Keep up the good work. I just fear this patch is a dead end.

There is still a lot of other issues you can contribute to.

Just one bigger project example: Comparing clang-format suggestions on
patches against checkpatch.pl suggestions are fine-tuning both of them to fit to
the actual kernel style.

Lukas

\
 
 \ /
  Last update: 2020-10-14 10:15    [W:0.181 / U:8.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site