linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Documentation: checkpatch: Add SYMBOLIC_PERMS message
@ 2021-09-04  7:29 Utkarsh Verma
  2021-09-04  7:46 ` Lukas Bulwahn
  0 siblings, 1 reply; 6+ messages in thread
From: Utkarsh Verma @ 2021-09-04  7:29 UTC (permalink / raw)
  To: Dwaipayan Ray
  Cc: Lukas Bulwahn, Joe Perches, Jonathan Corbet, linux-doc,
	linux-kernel, 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Documentation: checkpatch: Add SYMBOLIC_PERMS message
  2021-09-04  7:29 [PATCH] Documentation: checkpatch: Add SYMBOLIC_PERMS message Utkarsh Verma
@ 2021-09-04  7:46 ` Lukas Bulwahn
  2021-09-04  8:23   ` [PATCH v2] " Utkarsh Verma
  0 siblings, 1 reply; 6+ messages in thread
From: Lukas Bulwahn @ 2021-09-04  7:46 UTC (permalink / raw)
  To: Utkarsh Verma
  Cc: Dwaipayan Ray, Joe Perches, Jonathan Corbet,
	open list:DOCUMENTATION, Linux Kernel Mailing List

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] Documentation: checkpatch: Add SYMBOLIC_PERMS message
  2021-09-04  7:46 ` Lukas Bulwahn
@ 2021-09-04  8:23   ` Utkarsh Verma
  2021-09-09 17:02     ` Dwaipayan Ray
  0 siblings, 1 reply; 6+ messages in thread
From: Utkarsh Verma @ 2021-09-04  8:23 UTC (permalink / raw)
  To: Dwaipayan Ray
  Cc: Lukas Bulwahn, Joe Perches, Jonathan Corbet, linux-doc,
	linux-kernel, 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Documentation: checkpatch: Add SYMBOLIC_PERMS message
  2021-09-04  8:23   ` [PATCH v2] " Utkarsh Verma
@ 2021-09-09 17:02     ` Dwaipayan Ray
  2021-09-14 21:10       ` Jonathan Corbet
  0 siblings, 1 reply; 6+ messages in thread
From: Dwaipayan Ray @ 2021-09-09 17:02 UTC (permalink / raw)
  To: Utkarsh Verma
  Cc: Lukas Bulwahn, Joe Perches, Jonathan Corbet,
	open list:DOCUMENTATION, linux-kernel

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Documentation: checkpatch: Add SYMBOLIC_PERMS message
  2021-09-09 17:02     ` Dwaipayan Ray
@ 2021-09-14 21:10       ` Jonathan Corbet
  2021-09-14 22:28         ` Dwaipayan Ray
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Corbet @ 2021-09-14 21:10 UTC (permalink / raw)
  To: Dwaipayan Ray, Utkarsh Verma
  Cc: Lukas Bulwahn, Joe Perches, open list:DOCUMENTATION, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Documentation: checkpatch: Add SYMBOLIC_PERMS message
  2021-09-14 21:10       ` Jonathan Corbet
@ 2021-09-14 22:28         ` Dwaipayan Ray
  0 siblings, 0 replies; 6+ messages in thread
From: Dwaipayan Ray @ 2021-09-14 22:28 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Utkarsh Verma, Lukas Bulwahn, Joe Perches,
	open list:DOCUMENTATION, linux-kernel

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-14 22:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04  7:29 [PATCH] Documentation: checkpatch: Add SYMBOLIC_PERMS message Utkarsh Verma
2021-09-04  7:46 ` Lukas Bulwahn
2021-09-04  8:23   ` [PATCH v2] " Utkarsh Verma
2021-09-09 17:02     ` Dwaipayan Ray
2021-09-14 21:10       ` Jonathan Corbet
2021-09-14 22:28         ` Dwaipayan Ray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).