2020-05-11 02:23:29

by liwei (GF)

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

'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 <[email protected]>
---
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


2020-05-13 23:45:20

by Doug Anderson

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

Hi,

On Sun, May 10, 2020 at 7:18 PM Wei Li <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>

-Doug

2020-05-16 07:23:44

by liwei (GF)

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

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 <[email protected]>
>
> -Doug
>

Thanks,
Wei