linux-kernel - Re: [PATCH] KDB: Fix missing argument in dmesg command usage help

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: <CAD=FV=VRwiXFLz98Px_xpV4JLUMq_Ld_BTX8La0Oe5O9-d_=7Q@mail.gmail.com>
Date: Fri, 25 Oct 2024 15:37:49 -0700
From: Doug Anderson <dianders@...omium.org>
To: Nir Lichtman <nir@...htman.org>
Cc: jason.wessel@...driver.com, daniel.thompson@...aro.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KDB: Fix missing argument in dmesg command usage help

Hi,

On Tue, Oct 22, 2024 at 12:02 PM Nir Lichtman <nir@...htman.org> wrote:
>
> Problem: Currently when running "help" in KDB, it shows the "dmesg" command
> as having only a single argument, when in fact as can be seen in the
> implementation of the command (kdb_dmesg) it accepts two arguments
>
> Solution: Add the missing argument to the usage string of the "dmesg" command
>
> Signed-off-by: Nir Lichtman <nir@...htman.org>

It's not a huge deal, but above your Signed-off-by you could have added:

Suggested-by: Douglas Anderson <dianders@...omium.org>

...since this was my suggestion [1].

[1] https://lore.kernel.org/lkml/CAD=FV=VZ61XFb1Ks79BHr1jL1jwf_36wYXryy0ZXOz1xTQ9zOg@mail.gmail.com/

One other nit is that the ${SUBJECT} tag should have had the prefix
"kdb:" instead of "KDB:" (AKA not all caps) just based on doing a "git
log" on other changes to that file and seeing what they were doing.
It's not always obvious what the tag should be, but in this case I
think it's fairly consistent.

>  kernel/debug/kdb/kdb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index f5f7d7fb5936..5f56ade565a6 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -2827,7 +2827,7 @@ static kdbtab_t maintab[] = {
>  #if defined(CONFIG_PRINTK)
>         {       .name = "dmesg",
>                 .func = kdb_dmesg,
> -               .usage = "[lines]",
> +               .usage = "[lines] [adjust]",

Everything here is just a nit, so:

Reviewed-by: Douglas Anderson <dianders@...omium.org>

If you want, you could post a v2 adding the "Suggested-by" and fixing
the subject, but it's probably not a big deal. If Daniel cares then
he'll either ask you to post a v2 or fix it himself when applying. If
you do post a v2, you'll want to carry my "Reviewed-by" tag and add it
right above your "Signed-off-by" tag.

-Doug