[U-Boot] [PATCH v1 1/1] cmd: adding malloc, math, and strcmp commands to u-boot

Simon Glass sjg at chromium.org
Tue Nov 19 01:21:02 UTC 2019


Hi Vladimir,

On Mon, 18 Nov 2019 at 16:27, Vladimir Olovyannikov
<vladimir.olovyannikov at broadcom.com> wrote:
>
> cmd: adding malloc, math, and strcmp u-boot commands.

U-Boot

> - malloc  supports allocation of heap memory and free allocated memory
>           via u-boot command line.

U-Boot (please fix globally)

> - math    provides math commands such as "add", "sub", "mul", "div",
>           "shift", ability to convert dec->hex.
> - strcmp  provides string compare command feature for a script.
>
> All these commands are introduced to be used in u-boot scripts.
>
> Signed-off-by: Suji Velupiallai <suji.velupillai at broadcom.com>
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov at broadcom.com>
> ---
>  cmd/Kconfig  | 21 ++++++++++++++
>  cmd/Makefile |  4 +++
>  cmd/malloc.c | 54 ++++++++++++++++++++++++++++++++++++
>  cmd/math.c   | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  cmd/strcmp.c | 28 +++++++++++++++++++
>  5 files changed, 185 insertions(+)
>  create mode 100644 cmd/malloc.c
>  create mode 100644 cmd/math.c
>  create mode 100644 cmd/strcmp.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index cf982ff65e..f11903fe3d 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1286,6 +1286,20 @@ config CMD_ITEST
>         help
>           Return true/false on integer compare.
>
> +config CMD_MALLOC
> +       bool "malloc"
> +       default y
> +       help
> +         Supports allocation of heap memory and free allocated memory commands.
> +         These commands are used by u-boot scripts.
> +
> +config CMD_MATH
> +       bool "math"
> +       default y
> +       help
> +         Provides math commands such as add, sub, mul, div, shift,
> +         convert decimal to hex functionalities to be available in the script.
> +
>  config CMD_SOURCE
>         bool "source"
>         default y
> @@ -1301,6 +1315,13 @@ config CMD_SETEXPR
>           Also supports loading the value at a memory location into a variable.
>           If CONFIG_REGEX is enabled, setexpr also supports a gsub function.

I think this would be better as three patches.

>
> +config CMD_STRCMP
> +       bool "strcmp"
> +       default y
> +       help
> +         Provides string compare command feature to u-boot scripts.

U-Boot

> +
> +
>  endmenu
>
>  menu "Android support commands"
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 2d723ea0f0..942d60a0a2 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -164,6 +164,10 @@ obj-$(CONFIG_CMD_GPT) += gpt.o
>  obj-$(CONFIG_CMD_ETHSW) += ethsw.o
>  obj-$(CONFIG_CMD_AXI) += axi.o
>
> +obj-$(CONFIG_CMD_MALLOC) += malloc.o
> +obj-$(CONFIG_CMD_MATH) += math.o
> +obj-$(CONFIG_CMD_STRCMP) += strcmp.o
> +
>  # Power
>  obj-$(CONFIG_CMD_PMIC) += pmic.o
>  obj-$(CONFIG_CMD_REGULATOR) += regulator.o
> diff --git a/cmd/malloc.c b/cmd/malloc.c
> new file mode 100644
> index 0000000000..e11e030a59
> --- /dev/null
> +++ b/cmd/malloc.c
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019 Broadcom
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <environment.h>
> +#include <errno.h>
> +#include <malloc.h>
> +
> +static unsigned long get_value(const char *val)

Needs a comment

> +{
> +       char *env = env_get((char *)val);
> +
> +       if (env)
> +               return simple_strtoul(env, NULL, 16);
> +       else
> +               return simple_strtoul(val, NULL, 16);
> +}
> +
> +static int do_malloc(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> +{
> +       char numberbuf[32];
> +       void *mem;

const?

> +
> +       if (argc < 3)
> +               return cmd_usage(cmdtp);
> +
> +       mem = memalign(ARCH_DMA_MINALIGN, get_value(argv[2]));
> +       if (mem) {
> +               sprintf(numberbuf, "%08x", (unsigned int)mem);

I think (ulong) would be better

> +               env_set(argv[1], numberbuf);

blank line before return (please fix globally)

> +               return 0;
> +       }
> +       return -EINVAL;

You can't return errors from command functions. Try printing an error
and return CMD_RET_FAILURE instead.

> +}
> +
> +static int do_free(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> +{
> +       if (argc < 2)
> +               return cmd_usage(cmdtp);
> +
> +       free((void *)get_value(argv[1]));
> +       env_set(argv[1], "");
> +       return 0;
> +}
> +
> +U_BOOT_CMD(malloc, 3, 0, do_malloc,
> +          "Allocate memory from u-boot heap and store pointer in environment variable.",
> +          "target size\n");

Needs better help - e.g. list arguments

> +
> +U_BOOT_CMD(free, 2, 0, do_free,
> +          "Release memory from u-boot heap at target.", "target\n");

I wonder if it would be better to have a 'mem' command, then have 'mem
alloc', 'mem free', etc.?

> diff --git a/cmd/math.c b/cmd/math.c
> new file mode 100644
> index 0000000000..17de5ef70b
> --- /dev/null
> +++ b/cmd/math.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2010-2017 Broadcom
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <environment.h>
> +
> +unsigned long long simple_strtoull(const char *cp, char **endp,
> +                                  unsigned int base);

Use #include <vsprintf.h> instead

> +
> +static unsigned long long get_value(const char *val)

Function comment

> +{
> +       char *env = env_get((char *)val);
> +
> +       if (env)
> +               return simple_strtoull(env, NULL, 16);
> +       else
> +               return simple_strtoull(val, NULL, 16);
> +}
> +
> +static unsigned long long get_value_base10(const char *val)
> +{
> +       char *env = env_get((char *)val);
> +
> +       if (env)
> +               return simple_strtoull(env, NULL, 10);
> +       else
> +               return simple_strtoull(val, NULL, 10);
> +}
> +
> +/*
> + * Top level addenv command
> + */
> +#define IS_MATH_CMD(cmd, args) ((strcmp(argv[1], cmd) == 0) && \
> +                                 (argc == (args)))

Use a function instead of a macro.

> +
> +static int do_math(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +       char numberbuf[32];
> +       unsigned long long i = 0;
> +
> +       if (IS_MATH_CMD("add", 5))
> +               i = get_value(argv[3]) + get_value(argv[4]);

It might be better to do something like:

const char *left = NULL, *right = NULL;

if (argc > 3)
   left = argv[3];
...

Then you can use 'left' and 'right' below.

> +       else if (IS_MATH_CMD("sub", 5))
> +               i = get_value(argv[3]) - get_value(argv[4]);
> +       else if (IS_MATH_CMD("mul", 5))
> +               i = get_value(argv[3]) * get_value(argv[4]);
> +       else if (IS_MATH_CMD("div", 5))
> +               i = get_value(argv[3]) / get_value(argv[4]);
> +       else if (IS_MATH_CMD("shl", 5))
> +               i = get_value(argv[3]) << get_value(argv[4]);
> +       else if (IS_MATH_CMD("d2h", 4))
> +               i = get_value_base10(argv[3]);
> +       else
> +               return cmd_usage(cmdtp);
> +
> +       sprintf(numberbuf, "%llx", i);
> +       env_set(argv[2], numberbuf);
> +       return 0;
> +}
> +
> +U_BOOT_CMD(math, 5, 0, do_math,
> +          "Environment variable math.",
> +          "add a b c\n"
> +          "    - add b to c and store in a.\n"

But also b and c can be numbers, right? You should mention that.

> +          "math sub a b c\n"
> +          "    - subtract b from c and store in a.\n"
> +          "math mul a b c\n"
> +          "    - multiply b by c and store in a.\n"
> +          "math div a b c\n"
> +          "    - divide b by c and store in a.\n"
> +          "math shl a b c\n"
> +          "    - shift left b by c and store in a.\n"
> +          "math d2h a b\n"
> +          "    - Convert b from decimal to hex and store in a.\n"
> +);
> diff --git a/cmd/strcmp.c b/cmd/strcmp.c
> new file mode 100644
> index 0000000000..3abd270d40
> --- /dev/null
> +++ b/cmd/strcmp.c
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2010-2017 Broadcom
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +
> +static int do_strcmp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +       if (argc != 3)
> +               return cmd_usage(cmdtp);
> +
> +       if (strlen(argv[1]) != strlen(argv[2]))
> +               return CMD_RET_FAILURE;
> +
> +       /* Compare the temporary string to the given parameter */
> +       if (!strncmp(argv[1], argv[2], strlen(argv[2])))
> +               return CMD_RET_SUCCESS;
> +
> +       return CMD_RET_FAILURE;
> +}
> +
> +U_BOOT_CMD(strcmp, 3, 0, do_strcmp,
> +          "String to string comparison.",
> +          "[str] [str]\n"

str1 str2, I think

> +          "    - Returns true if str are same, else returns false.\n"
> +);
> --
> 2.17.1
>

Regards,
Simon


More information about the U-Boot mailing list