linux-kernel - Re: [PATCH V2] : kernel-chktaint : Fixed space ,cosmetic change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3ca78f7a-4431-b345-85e4-eb07fa8a4038@leemhuis.info>
Date:   Wed, 25 Mar 2020 12:23:25 +0100
From:   Thorsten Leemhuis <linux@...mhuis.info>
To:     Bhaskar Chowdhury <unixbhaskar@...il.com>, rdunlap@...radead.org,
        joe@...ches.com
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] : kernel-chktaint : Fixed space ,cosmetic change

Lo! Thx for taking a look at this, but it seems a few small details need
to be improved before this can be applied. A few general issues first:

* your patch v2 is afaics on top of the first patch; when doing further
revisions please merge all your changes into one patch so everything you
want to do can be done in one commit (and people that see this mail
out-of-context can easily gasp what this is all about).

* tools/debugging/kernel-chktaint was added via the docs tree, thus I
think it's best if this change takes the same route, so please CC the
docs maintainer Jonathan Corbet <corbet@....net>
 and linux-doc@...r.kernel.org
 – I might be wrong with that line of thought, but Jonathan will know
for sure.

Am 24.03.20 um 23:59 schrieb Bhaskar Chowdhury:
> Space bwtween

Typo

> the words is fixed at the bottom of the file,sentence

Missing space after the comma.

> starting with "Documentation....."
> 
> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@...il.com>
> ---
>  tools/debugging/kernel-chktaint | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/debugging/kernel-chktaint b/tools/debugging/kernel-chktaint
> index 74fd3282aa1b..1af06bc0e667 100755
> --- a/tools/debugging/kernel-chktaint
> +++ b/tools/debugging/kernel-chktaint
> @@ -198,6 +198,6 @@ fi
>  echo "Raw taint value as int/string: $taint/'$out'"

This used to be the last time. Did you move it upwards on purpose?

>  echo

If you add a blank line here you IMHO might want to add one above the
""Raw taint value" as well.

>  echo "For a more detailed explanation of the various taint flags see below pointers:"
> -echo "1) Documentation/admin-guide/tainted-kernels.rst in  the Linux kernel sources"
> +echo "1) Documentation/admin-guide/tainted-kernels.rst in the Linux kernel sources"
>  echo "2)  https://kernel.org/doc/html/latest/admin-guide/tainted-kernels.html"
There are two spaces here as well: "2)  https".

And I for one dislike the "1)" "2)" style, as in the end it's the same
file in different locations. How about a text like this instead:

```
For a more detailed explanation of the various taint flags see
Documentation/admin-guide/tainted-kernels.rst in the Linux sources which
is also available online as rendered webpage at
https://kernel.org/doc/html/latest/admin-guide/tainted-kernels.html"
```

Feel free to improve on that, it's just a suggestion. I for one wonder
if this cosmetic change is worth all of this, but no worries.

Note, you also want to change tainted-kernels.rst, as in once place it
shows the output of this tool (which is one more reason to route this
via the docs tree).

Ciao, Thorsten