* [PATCH v3] kdb: Refactor env variables get/set code
@ 2021-02-08 8:02 Sumit Garg
2021-03-23 6:38 ` Sumit Garg
2021-04-14 12:46 ` Daniel Thompson
0 siblings, 2 replies; 3+ messages in thread
From: Sumit Garg @ 2021-02-08 8:02 UTC (permalink / raw)
To: kgdb-bugreport
Cc: jason.wessel, daniel.thompson, dianders, linux-kernel, Sumit Garg
Add two new kdb environment access methods as kdb_setenv() and
kdb_printenv() in order to abstract out environment access code
from kdb command functions.
Also, replace (char *)0 with NULL as an initializer for environment
variables array.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v3:
- Remove redundant '\0' char assignment.
- Pick up Doug's review tag.
Changes in v2:
- Get rid of code motion to separate kdb_env.c file.
- Replace (char *)0 with NULL.
- Use kernel-doc style function comments.
- s/kdb_prienv/kdb_printenv/
kernel/debug/kdb/kdb_main.c | 164 ++++++++++++++++++++++++--------------------
1 file changed, 91 insertions(+), 73 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 588062a..69b8f55 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -142,40 +142,40 @@ static const int __nkdb_err = ARRAY_SIZE(kdbmsgs);
static char *__env[] = {
#if defined(CONFIG_SMP)
- "PROMPT=[%d]kdb> ",
+ "PROMPT=[%d]kdb> ",
#else
- "PROMPT=kdb> ",
+ "PROMPT=kdb> ",
#endif
- "MOREPROMPT=more> ",
- "RADIX=16",
- "MDCOUNT=8", /* lines of md output */
- KDB_PLATFORM_ENV,
- "DTABCOUNT=30",
- "NOSECT=1",
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
- (char *)0,
+ "MOREPROMPT=more> ",
+ "RADIX=16",
+ "MDCOUNT=8", /* lines of md output */
+ KDB_PLATFORM_ENV,
+ "DTABCOUNT=30",
+ "NOSECT=1",
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
};
static const int __nenv = ARRAY_SIZE(__env);
@@ -318,6 +318,63 @@ int kdbgetintenv(const char *match, int *value)
}
/*
+ * kdb_setenv() - Alter an existing environment variable or create a new one.
+ * @var: Name of the variable
+ * @val: Value of the variable
+ *
+ * Return: Zero on success, a kdb diagnostic on failure.
+ */
+static int kdb_setenv(const char *var, const char *val)
+{
+ int i;
+ char *ep;
+ size_t varlen, vallen;
+
+ varlen = strlen(var);
+ vallen = strlen(val);
+ ep = kdballocenv(varlen + vallen + 2);
+ if (ep == (char *)0)
+ return KDB_ENVBUFFULL;
+
+ sprintf(ep, "%s=%s", var, val);
+
+ for (i = 0; i < __nenv; i++) {
+ if (__env[i]
+ && ((strncmp(__env[i], var, varlen) == 0)
+ && ((__env[i][varlen] == '\0')
+ || (__env[i][varlen] == '=')))) {
+ __env[i] = ep;
+ return 0;
+ }
+ }
+
+ /*
+ * Wasn't existing variable. Fit into slot.
+ */
+ for (i = 0; i < __nenv-1; i++) {
+ if (__env[i] == (char *)0) {
+ __env[i] = ep;
+ return 0;
+ }
+ }
+
+ return KDB_ENVFULL;
+}
+
+/*
+ * kdb_printenv() - Display the current environment variables.
+ */
+static void kdb_printenv(void)
+{
+ int i;
+
+ for (i = 0; i < __nenv; i++) {
+ if (__env[i])
+ kdb_printf("%s\n", __env[i]);
+ }
+}
+
+/*
* kdbgetularg - This function will convert a numeric string into an
* unsigned long value.
* Parameters:
@@ -374,10 +431,6 @@ int kdbgetu64arg(const char *arg, u64 *value)
*/
int kdb_set(int argc, const char **argv)
{
- int i;
- char *ep;
- size_t varlen, vallen;
-
/*
* we can be invoked two ways:
* set var=value argv[1]="var", argv[2]="value"
@@ -422,37 +475,7 @@ int kdb_set(int argc, const char **argv)
* Tokenizer squashed the '=' sign. argv[1] is variable
* name, argv[2] = value.
*/
- varlen = strlen(argv[1]);
- vallen = strlen(argv[2]);
- ep = kdballocenv(varlen + vallen + 2);
- if (ep == (char *)0)
- return KDB_ENVBUFFULL;
-
- sprintf(ep, "%s=%s", argv[1], argv[2]);
-
- ep[varlen+vallen+1] = '\0';
-
- for (i = 0; i < __nenv; i++) {
- if (__env[i]
- && ((strncmp(__env[i], argv[1], varlen) == 0)
- && ((__env[i][varlen] == '\0')
- || (__env[i][varlen] == '=')))) {
- __env[i] = ep;
- return 0;
- }
- }
-
- /*
- * Wasn't existing variable. Fit into slot.
- */
- for (i = 0; i < __nenv-1; i++) {
- if (__env[i] == (char *)0) {
- __env[i] = ep;
- return 0;
- }
- }
-
- return KDB_ENVFULL;
+ return kdb_setenv(argv[1], argv[2]);
}
static int kdb_check_regs(void)
@@ -2055,12 +2078,7 @@ static int kdb_lsmod(int argc, const char **argv)
static int kdb_env(int argc, const char **argv)
{
- int i;
-
- for (i = 0; i < __nenv; i++) {
- if (__env[i])
- kdb_printf("%s\n", __env[i]);
- }
+ kdb_printenv();
if (KDB_DEBUG(MASK))
kdb_printf("KDBDEBUG=0x%x\n",
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] kdb: Refactor env variables get/set code
2021-02-08 8:02 [PATCH v3] kdb: Refactor env variables get/set code Sumit Garg
@ 2021-03-23 6:38 ` Sumit Garg
2021-04-14 12:46 ` Daniel Thompson
1 sibling, 0 replies; 3+ messages in thread
From: Sumit Garg @ 2021-03-23 6:38 UTC (permalink / raw)
To: Daniel Thompson
Cc: Jason Wessel, Douglas Anderson, Linux Kernel Mailing List,
kgdb-bugreport
Hi Daniel,
On Mon, 8 Feb 2021 at 13:32, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Add two new kdb environment access methods as kdb_setenv() and
> kdb_printenv() in order to abstract out environment access code
> from kdb command functions.
>
> Also, replace (char *)0 with NULL as an initializer for environment
> variables array.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v3:
> - Remove redundant '\0' char assignment.
> - Pick up Doug's review tag.
>
> Changes in v2:
> - Get rid of code motion to separate kdb_env.c file.
> - Replace (char *)0 with NULL.
> - Use kernel-doc style function comments.
> - s/kdb_prienv/kdb_printenv/
>
> kernel/debug/kdb/kdb_main.c | 164 ++++++++++++++++++++++++--------------------
> 1 file changed, 91 insertions(+), 73 deletions(-)
>
Do you have any further comments on this? If no, can you pick this up as well?
-Sumit
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 588062a..69b8f55 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -142,40 +142,40 @@ static const int __nkdb_err = ARRAY_SIZE(kdbmsgs);
>
> static char *__env[] = {
> #if defined(CONFIG_SMP)
> - "PROMPT=[%d]kdb> ",
> + "PROMPT=[%d]kdb> ",
> #else
> - "PROMPT=kdb> ",
> + "PROMPT=kdb> ",
> #endif
> - "MOREPROMPT=more> ",
> - "RADIX=16",
> - "MDCOUNT=8", /* lines of md output */
> - KDB_PLATFORM_ENV,
> - "DTABCOUNT=30",
> - "NOSECT=1",
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> + "MOREPROMPT=more> ",
> + "RADIX=16",
> + "MDCOUNT=8", /* lines of md output */
> + KDB_PLATFORM_ENV,
> + "DTABCOUNT=30",
> + "NOSECT=1",
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> };
>
> static const int __nenv = ARRAY_SIZE(__env);
> @@ -318,6 +318,63 @@ int kdbgetintenv(const char *match, int *value)
> }
>
> /*
> + * kdb_setenv() - Alter an existing environment variable or create a new one.
> + * @var: Name of the variable
> + * @val: Value of the variable
> + *
> + * Return: Zero on success, a kdb diagnostic on failure.
> + */
> +static int kdb_setenv(const char *var, const char *val)
> +{
> + int i;
> + char *ep;
> + size_t varlen, vallen;
> +
> + varlen = strlen(var);
> + vallen = strlen(val);
> + ep = kdballocenv(varlen + vallen + 2);
> + if (ep == (char *)0)
> + return KDB_ENVBUFFULL;
> +
> + sprintf(ep, "%s=%s", var, val);
> +
> + for (i = 0; i < __nenv; i++) {
> + if (__env[i]
> + && ((strncmp(__env[i], var, varlen) == 0)
> + && ((__env[i][varlen] == '\0')
> + || (__env[i][varlen] == '=')))) {
> + __env[i] = ep;
> + return 0;
> + }
> + }
> +
> + /*
> + * Wasn't existing variable. Fit into slot.
> + */
> + for (i = 0; i < __nenv-1; i++) {
> + if (__env[i] == (char *)0) {
> + __env[i] = ep;
> + return 0;
> + }
> + }
> +
> + return KDB_ENVFULL;
> +}
> +
> +/*
> + * kdb_printenv() - Display the current environment variables.
> + */
> +static void kdb_printenv(void)
> +{
> + int i;
> +
> + for (i = 0; i < __nenv; i++) {
> + if (__env[i])
> + kdb_printf("%s\n", __env[i]);
> + }
> +}
> +
> +/*
> * kdbgetularg - This function will convert a numeric string into an
> * unsigned long value.
> * Parameters:
> @@ -374,10 +431,6 @@ int kdbgetu64arg(const char *arg, u64 *value)
> */
> int kdb_set(int argc, const char **argv)
> {
> - int i;
> - char *ep;
> - size_t varlen, vallen;
> -
> /*
> * we can be invoked two ways:
> * set var=value argv[1]="var", argv[2]="value"
> @@ -422,37 +475,7 @@ int kdb_set(int argc, const char **argv)
> * Tokenizer squashed the '=' sign. argv[1] is variable
> * name, argv[2] = value.
> */
> - varlen = strlen(argv[1]);
> - vallen = strlen(argv[2]);
> - ep = kdballocenv(varlen + vallen + 2);
> - if (ep == (char *)0)
> - return KDB_ENVBUFFULL;
> -
> - sprintf(ep, "%s=%s", argv[1], argv[2]);
> -
> - ep[varlen+vallen+1] = '\0';
> -
> - for (i = 0; i < __nenv; i++) {
> - if (__env[i]
> - && ((strncmp(__env[i], argv[1], varlen) == 0)
> - && ((__env[i][varlen] == '\0')
> - || (__env[i][varlen] == '=')))) {
> - __env[i] = ep;
> - return 0;
> - }
> - }
> -
> - /*
> - * Wasn't existing variable. Fit into slot.
> - */
> - for (i = 0; i < __nenv-1; i++) {
> - if (__env[i] == (char *)0) {
> - __env[i] = ep;
> - return 0;
> - }
> - }
> -
> - return KDB_ENVFULL;
> + return kdb_setenv(argv[1], argv[2]);
> }
>
> static int kdb_check_regs(void)
> @@ -2055,12 +2078,7 @@ static int kdb_lsmod(int argc, const char **argv)
>
> static int kdb_env(int argc, const char **argv)
> {
> - int i;
> -
> - for (i = 0; i < __nenv; i++) {
> - if (__env[i])
> - kdb_printf("%s\n", __env[i]);
> - }
> + kdb_printenv();
>
> if (KDB_DEBUG(MASK))
> kdb_printf("KDBDEBUG=0x%x\n",
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] kdb: Refactor env variables get/set code
2021-02-08 8:02 [PATCH v3] kdb: Refactor env variables get/set code Sumit Garg
2021-03-23 6:38 ` Sumit Garg
@ 2021-04-14 12:46 ` Daniel Thompson
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Thompson @ 2021-04-14 12:46 UTC (permalink / raw)
To: Sumit Garg; +Cc: kgdb-bugreport, jason.wessel, dianders, linux-kernel
On Mon, Feb 08, 2021 at 01:32:22PM +0530, Sumit Garg wrote:
>
> Add two new kdb environment access methods as kdb_setenv() and
> kdb_printenv() in order to abstract out environment access code
> from kdb command functions.
>
> Also, replace (char *)0 with NULL as an initializer for environment
> variables array.
Neither (char *)0 nor NULL are great initializers since, for static
data, these are the default value anyway. Better to just give the array
a explicit dimension and be done.
However... that's a fairly small change and I can fix it up when
applying this.
Daniel.
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v3:
> - Remove redundant '\0' char assignment.
> - Pick up Doug's review tag.
>
> Changes in v2:
> - Get rid of code motion to separate kdb_env.c file.
> - Replace (char *)0 with NULL.
> - Use kernel-doc style function comments.
> - s/kdb_prienv/kdb_printenv/
>
> kernel/debug/kdb/kdb_main.c | 164 ++++++++++++++++++++++++--------------------
> 1 file changed, 91 insertions(+), 73 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 588062a..69b8f55 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -142,40 +142,40 @@ static const int __nkdb_err = ARRAY_SIZE(kdbmsgs);
>
> static char *__env[] = {
> #if defined(CONFIG_SMP)
> - "PROMPT=[%d]kdb> ",
> + "PROMPT=[%d]kdb> ",
> #else
> - "PROMPT=kdb> ",
> + "PROMPT=kdb> ",
> #endif
> - "MOREPROMPT=more> ",
> - "RADIX=16",
> - "MDCOUNT=8", /* lines of md output */
> - KDB_PLATFORM_ENV,
> - "DTABCOUNT=30",
> - "NOSECT=1",
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> - (char *)0,
> + "MOREPROMPT=more> ",
> + "RADIX=16",
> + "MDCOUNT=8", /* lines of md output */
> + KDB_PLATFORM_ENV,
> + "DTABCOUNT=30",
> + "NOSECT=1",
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> };
>
> static const int __nenv = ARRAY_SIZE(__env);
> @@ -318,6 +318,63 @@ int kdbgetintenv(const char *match, int *value)
> }
>
> /*
> + * kdb_setenv() - Alter an existing environment variable or create a new one.
> + * @var: Name of the variable
> + * @val: Value of the variable
> + *
> + * Return: Zero on success, a kdb diagnostic on failure.
> + */
> +static int kdb_setenv(const char *var, const char *val)
> +{
> + int i;
> + char *ep;
> + size_t varlen, vallen;
> +
> + varlen = strlen(var);
> + vallen = strlen(val);
> + ep = kdballocenv(varlen + vallen + 2);
> + if (ep == (char *)0)
> + return KDB_ENVBUFFULL;
> +
> + sprintf(ep, "%s=%s", var, val);
> +
> + for (i = 0; i < __nenv; i++) {
> + if (__env[i]
> + && ((strncmp(__env[i], var, varlen) == 0)
> + && ((__env[i][varlen] == '\0')
> + || (__env[i][varlen] == '=')))) {
> + __env[i] = ep;
> + return 0;
> + }
> + }
> +
> + /*
> + * Wasn't existing variable. Fit into slot.
> + */
> + for (i = 0; i < __nenv-1; i++) {
> + if (__env[i] == (char *)0) {
> + __env[i] = ep;
> + return 0;
> + }
> + }
> +
> + return KDB_ENVFULL;
> +}
> +
> +/*
> + * kdb_printenv() - Display the current environment variables.
> + */
> +static void kdb_printenv(void)
> +{
> + int i;
> +
> + for (i = 0; i < __nenv; i++) {
> + if (__env[i])
> + kdb_printf("%s\n", __env[i]);
> + }
> +}
> +
> +/*
> * kdbgetularg - This function will convert a numeric string into an
> * unsigned long value.
> * Parameters:
> @@ -374,10 +431,6 @@ int kdbgetu64arg(const char *arg, u64 *value)
> */
> int kdb_set(int argc, const char **argv)
> {
> - int i;
> - char *ep;
> - size_t varlen, vallen;
> -
> /*
> * we can be invoked two ways:
> * set var=value argv[1]="var", argv[2]="value"
> @@ -422,37 +475,7 @@ int kdb_set(int argc, const char **argv)
> * Tokenizer squashed the '=' sign. argv[1] is variable
> * name, argv[2] = value.
> */
> - varlen = strlen(argv[1]);
> - vallen = strlen(argv[2]);
> - ep = kdballocenv(varlen + vallen + 2);
> - if (ep == (char *)0)
> - return KDB_ENVBUFFULL;
> -
> - sprintf(ep, "%s=%s", argv[1], argv[2]);
> -
> - ep[varlen+vallen+1] = '\0';
> -
> - for (i = 0; i < __nenv; i++) {
> - if (__env[i]
> - && ((strncmp(__env[i], argv[1], varlen) == 0)
> - && ((__env[i][varlen] == '\0')
> - || (__env[i][varlen] == '=')))) {
> - __env[i] = ep;
> - return 0;
> - }
> - }
> -
> - /*
> - * Wasn't existing variable. Fit into slot.
> - */
> - for (i = 0; i < __nenv-1; i++) {
> - if (__env[i] == (char *)0) {
> - __env[i] = ep;
> - return 0;
> - }
> - }
> -
> - return KDB_ENVFULL;
> + return kdb_setenv(argv[1], argv[2]);
> }
>
> static int kdb_check_regs(void)
> @@ -2055,12 +2078,7 @@ static int kdb_lsmod(int argc, const char **argv)
>
> static int kdb_env(int argc, const char **argv)
> {
> - int i;
> -
> - for (i = 0; i < __nenv; i++) {
> - if (__env[i])
> - kdb_printf("%s\n", __env[i]);
> - }
> + kdb_printenv();
>
> if (KDB_DEBUG(MASK))
> kdb_printf("KDBDEBUG=0x%x\n",
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-14 12:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-08 8:02 [PATCH v3] kdb: Refactor env variables get/set code Sumit Garg
2021-03-23 6:38 ` Sumit Garg
2021-04-14 12:46 ` Daniel Thompson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.