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

[PATCH v2] 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>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
v1 -> v2:
 - Fix lack of braces.

 kernel/debug/kdb/kdb_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4fc43fb17127..75b798340300 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -423,6 +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;
 	}
 
 	/*
-- 
2.17.1

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

[Daniel Thompson]

On Sat, May 16, 2020 at 05:26:06PM +0800, Wei Li 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>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

I took a quick look at this and find myself thinking of KDBFLAGS as
something of a misfeature.

I think I'd rather get kdb_env to show the value we wrote into
KDBDEBUG.

Sure this means we cannot use KDBDEBUG to look at the least significant
16-bits but I think anyone who is debugging kdb itself should know
enough to use `md4c1 kdb_flags` to read those anyway.


Daniel.



> ---
> v1 -> v2:
>  - Fix lack of braces.
> 
>  kernel/debug/kdb/kdb_main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 4fc43fb17127..75b798340300 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -423,6 +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;
>  	}
>  
>  	/*
> -- 
> 2.17.1
> 

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

[liwei (GF)]

Hi Daniel,

On 2020/5/19 19:40, Daniel Thompson wrote:
> On Sat, May 16, 2020 at 05:26:06PM +0800, Wei Li 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>
>> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> I took a quick look at this and find myself thinking of KDBFLAGS as
> something of a misfeature.
> 
> I think I'd rather get kdb_env to show the value we wrote into
> KDBDEBUG.
> 
> Sure this means we cannot use KDBDEBUG to look at the least significant
> 16-bits but I think anyone who is debugging kdb itself should know
> enough to use `md4c1 kdb_flags` to read those anyway.
> 
> 

Agree. That will be more clear with no confusion. Currently,
KDBFLAGS will be shown only when KDBDEBUG is set, that is weird too.
So, let's just make it simple.
I can fix it as you suggested in the next post.

Thanks,
Wei