[PATCH] kdb: Make the internal env 'KDBFLAGS' undefinable

[PATCH] kdb: Make the internal env 'KDBFLAGS' undefinable

[Wei Li]

'KDBFLAGS' is an internal variable of kdb, it is combined by 'KDBDEBUG'
and state flags. But the user can define an environment variable named
'KDBFLAGS' too, so let's make it undefinable to avoid confusion.

Signed-off-by: Wei Li <liwei391@huawei.com>
---
 kernel/debug/kdb/kdb_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4fc43fb17127..d3d060136821 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -423,7 +423,8 @@ int kdb_set(int argc, const char **argv)
 			| (debugflags << KDB_DEBUG_FLAG_SHIFT);
 
 		return 0;
-	}
+	} else if (strcmp(argv[1], "KDBFLAGS") == 0)
+		return KDB_NOPERM;
 
 	/*
 	 * Tokenizer squashed the '=' sign.  argv[1] is variable
-- 
2.17.1

Re: [PATCH] kdb: Make the internal env 'KDBFLAGS' undefinable

[Doug Anderson]

Hi,

On Sun, May 10, 2020 at 7:18 PM Wei Li <liwei391@huawei.com> wrote:
>
> 'KDBFLAGS' is an internal variable of kdb, it is combined by 'KDBDEBUG'
> and state flags. But the user can define an environment variable named
> 'KDBFLAGS' too, so let's make it undefinable to avoid confusion.
>
> Signed-off-by: Wei Li <liwei391@huawei.com>
> ---
>  kernel/debug/kdb/kdb_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 4fc43fb17127..d3d060136821 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -423,7 +423,8 @@ int kdb_set(int argc, const char **argv)
>                         | (debugflags << KDB_DEBUG_FLAG_SHIFT);
>
>                 return 0;
> -       }
> +       } else if (strcmp(argv[1], "KDBFLAGS") == 0)
> +               return KDB_NOPERM;

One slight nit is that my personal preference is that if one half of
an "if/else" needs braces then both halves should have braces.  I
don't know what Daniel and Jason's policies are, though.  In any case,
not that I've ever used the KDBDEBUG functionality, but your change
seems sane.  Without it if I set "KDBDEBUG" and "KDBFLAGS" and then
type "env" I see the flags listed twice, but one is real and one is
fake.

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

-Doug

Re: [PATCH] kdb: Make the internal env 'KDBFLAGS' undefinable

[liwei (GF)]

Hi Douglas,

On 2020/5/14 7:41, Doug Anderson wrote:

>> -       }
>> +       } else if (strcmp(argv[1], "KDBFLAGS") == 0)
>> +               return KDB_NOPERM;
> 
> One slight nit is that my personal preference is that if one half of
> an "if/else" needs braces then both halves should have braces.  I
Thanks for spotting it. Refer to Documentation/process/coding-style.rst, i
will fix it in the v2.

> don't know what Daniel and Jason's policies are, though.  In any case,
> not that I've ever used the KDBDEBUG functionality, but your change
> seems sane.  Without it if I set "KDBDEBUG" and "KDBFLAGS" and then
> type "env" I see the flags listed twice, but one is real and one is
> fake.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> -Doug
> 

Thanks,
Wei