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: <CAFA6WYPPTivcLW8D+_nRdc4M1FLr21cbNt81BXYgbb7T3odaWw@mail.gmail.com>
Date:   Tue, 23 Mar 2021 12:08:09 +0530
From:   Sumit Garg <sumit.garg@...aro.org>
To:     Daniel Thompson <daniel.thompson@...aro.org>
Cc:     Jason Wessel <jason.wessel@...driver.com>,
        Douglas Anderson <dianders@...omium.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        kgdb-bugreport@...ts.sourceforge.net
Subject: Re: [PATCH v3] kdb: Refactor env variables get/set code

Hi Daniel,

On Mon, 8 Feb 2021 at 13:32, Sumit Garg <sumit.garg@...aro.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@...aro.org>
> Reviewed-by: Douglas Anderson <dianders@...omium.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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ