[PATCH] Documentation: checkpatch: Add SYMBOLIC_PERMS message

[PATCH] Documentation: checkpatch: Add SYMBOLIC_PERMS message

[Utkarsh Verma]

Add a new message type SYMBOLIC_PERMS under the 'Permissions'
subsection. Octal permission bits are easier to read and understand
instead of their symbolic macro names.

Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index f0956e9ea2d8..01105e9c89de 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -957,6 +957,17 @@ Permissions
     Permission bits should use 4 digit octal permissions (like 0700 or 0444).
     Avoid using any other base like decimal.
 
+  **SYMBOLIC_PERMS**
+    Permission bits in the octal form are more readable and easier to
+    understand than their symbolic counterparts because many command-line
+    tools use this notation only. Experienced kernel developers have been using
+    this traditional Unix permission bits for decades and so they find it
+    easier to understand the octal notation than the symbolic macros.
+    Also, it is harder to read S_IWUSR|S_IRUGO than 0644, which obscures the
+    developer's intent rather than clarifying it.
+
+    See: https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/
+
 
 Spacing and Brackets
 --------------------
-- 
2.25.1

Re: [PATCH] Documentation: checkpatch: Add SYMBOLIC_PERMS message

[Lukas Bulwahn]

On Sat, Sep 4, 2021 at 9:30 AM Utkarsh Verma <utkarshverma294@gmail.com> wrote:
>
> Add a new message type SYMBOLIC_PERMS under the 'Permissions'
> subsection. Octal permission bits are easier to read and understand
> instead of their symbolic macro names.
>
> Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
> ---
>  Documentation/dev-tools/checkpatch.rst | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> index f0956e9ea2d8..01105e9c89de 100644
> --- a/Documentation/dev-tools/checkpatch.rst
> +++ b/Documentation/dev-tools/checkpatch.rst
> @@ -957,6 +957,17 @@ Permissions
>      Permission bits should use 4 digit octal permissions (like 0700 or 0444).
>      Avoid using any other base like decimal.
>
> +  **SYMBOLIC_PERMS**
> +    Permission bits in the octal form are more readable and easier to
> +    understand than their symbolic counterparts because many command-line
> +    tools use this notation only. Experienced kernel developers have been using
> +    this traditional Unix permission bits for decades and so they find it
> +    easier to understand the octal notation than the symbolic macros.
> +    Also, it is harder to read S_IWUSR|S_IRUGO than 0644, which obscures the
> +    developer's intent rather than clarifying it.

Just a quick stylistic nit:

s/Also, it is harder to read /For example, it is harder to read/

Other than that:

Acked-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Reviewed-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>

Feel free to send a quick v2 for my nitpicking, and apply the tags
from this email.

Lukas

> +
> +    See: https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/
> +
>
>  Spacing and Brackets
>  --------------------
> --
> 2.25.1
>

[PATCH v2] Documentation: checkpatch: Add SYMBOLIC_PERMS message

[Utkarsh Verma]

Add a new message type SYMBOLIC_PERMS under the 'Permissions'
subsection. Octal permission bits are easier to read and understand
instead of their symbolic macro names.

Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
Acked-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Reviewed-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index f0956e9ea2d8..41037594ec24 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -957,6 +957,17 @@ Permissions
     Permission bits should use 4 digit octal permissions (like 0700 or 0444).
     Avoid using any other base like decimal.
 
+  **SYMBOLIC_PERMS**
+    Permission bits in the octal form are more readable and easier to
+    understand than their symbolic counterparts because many command-line
+    tools use this notation only. Experienced kernel developers have been using
+    this traditional Unix permission bits for decades and so they find it
+    easier to understand the octal notation than the symbolic macros.
+    For example, it is harder to read S_IWUSR|S_IRUGO than 0644, which
+    obscures the developer's intent rather than clarifying it.
+
+    See: https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/
+
 
 Spacing and Brackets
 --------------------
-- 
2.25.1

Re: [PATCH v2] Documentation: checkpatch: Add SYMBOLIC_PERMS message

[Dwaipayan Ray]

On Sat, Sep 4, 2021 at 1:53 PM Utkarsh Verma <utkarshverma294@gmail.com> wrote:
>
> Add a new message type SYMBOLIC_PERMS under the 'Permissions'
> subsection. Octal permission bits are easier to read and understand
> instead of their symbolic macro names.
>
> Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
> Acked-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Reviewed-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> ---
>  Documentation/dev-tools/checkpatch.rst | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> index f0956e9ea2d8..41037594ec24 100644
> --- a/Documentation/dev-tools/checkpatch.rst
> +++ b/Documentation/dev-tools/checkpatch.rst
> @@ -957,6 +957,17 @@ Permissions
>      Permission bits should use 4 digit octal permissions (like 0700 or 0444).
>      Avoid using any other base like decimal.
>
> +  **SYMBOLIC_PERMS**
> +    Permission bits in the octal form are more readable and easier to
> +    understand than their symbolic counterparts because many command-line
> +    tools use this notation only. Experienced kernel developers have been using

Let's remove "only".

> +    this traditional Unix permission bits for decades and so they find it

Maybe you meant "these" here.

With these changes made,
Acked-by: Dwaipayan Ray <dwaipayanray1@gmail.com>

> +    easier to understand the octal notation than the symbolic macros.
> +    For example, it is harder to read S_IWUSR|S_IRUGO than 0644, which
> +    obscures the developer's intent rather than clarifying it.
> +
> +    See: https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/
> +
>
>  Spacing and Brackets
>  --------------------
> --
> 2.25.1
>

Re: [PATCH v2] Documentation: checkpatch: Add SYMBOLIC_PERMS message

[Jonathan Corbet]

Dwaipayan Ray <dwaipayanray1@gmail.com> writes:

> On Sat, Sep 4, 2021 at 1:53 PM Utkarsh Verma <utkarshverma294@gmail.com> wrote:
>>
>> Add a new message type SYMBOLIC_PERMS under the 'Permissions'
>> subsection. Octal permission bits are easier to read and understand
>> instead of their symbolic macro names.
>>
>> Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
>> Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
>> Acked-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
>> Reviewed-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
>> ---
>>  Documentation/dev-tools/checkpatch.rst | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
>> index f0956e9ea2d8..41037594ec24 100644
>> --- a/Documentation/dev-tools/checkpatch.rst
>> +++ b/Documentation/dev-tools/checkpatch.rst
>> @@ -957,6 +957,17 @@ Permissions
>>      Permission bits should use 4 digit octal permissions (like 0700 or 0444).
>>      Avoid using any other base like decimal.
>>
>> +  **SYMBOLIC_PERMS**
>> +    Permission bits in the octal form are more readable and easier to
>> +    understand than their symbolic counterparts because many command-line
>> +    tools use this notation only. Experienced kernel developers have been using
>
> Let's remove "only".
>
>> +    this traditional Unix permission bits for decades and so they find it
>
> Maybe you meant "these" here.
>
> With these changes made,
> Acked-by: Dwaipayan Ray <dwaipayanray1@gmail.com>

I took the liberty of apply the patch with those changes made.

Thanks,

jon

Re: [PATCH v2] Documentation: checkpatch: Add SYMBOLIC_PERMS message

[Dwaipayan Ray]

On Wed, Sep 15, 2021 at 2:40 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> Dwaipayan Ray <dwaipayanray1@gmail.com> writes:
>
> > On Sat, Sep 4, 2021 at 1:53 PM Utkarsh Verma <utkarshverma294@gmail.com> wrote:
> >>
> >> Add a new message type SYMBOLIC_PERMS under the 'Permissions'
> >> subsection. Octal permission bits are easier to read and understand
> >> instead of their symbolic macro names.
> >>
> >> Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> >> Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
> >> Acked-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> >> Reviewed-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> >> ---
> >>  Documentation/dev-tools/checkpatch.rst | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> >> index f0956e9ea2d8..41037594ec24 100644
> >> --- a/Documentation/dev-tools/checkpatch.rst
> >> +++ b/Documentation/dev-tools/checkpatch.rst
> >> @@ -957,6 +957,17 @@ Permissions
> >>      Permission bits should use 4 digit octal permissions (like 0700 or 0444).
> >>      Avoid using any other base like decimal.
> >>
> >> +  **SYMBOLIC_PERMS**
> >> +    Permission bits in the octal form are more readable and easier to
> >> +    understand than their symbolic counterparts because many command-line
> >> +    tools use this notation only. Experienced kernel developers have been using
> >
> > Let's remove "only".
> >
> >> +    this traditional Unix permission bits for decades and so they find it
> >
> > Maybe you meant "these" here.
> >
> > With these changes made,
> > Acked-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
>
> I took the liberty of apply the patch with those changes made.
>

Thanks Jonathan.

Utkarsh, you can start working on your next patches after submitting, you don't
have to wait for the existing patches to be first accepted. They will follow
the same review -> changes -> review cycle until they are good for
acceptance.

Like lukas said, try preparing a batch of say 3 to 5 rules and let's
review it and get it in.

Thanks,
Dwaipayan.