Hi Simon, Thank you for reviewing. I will split the patch as you suggested, and resubmit.
Vladimir
-----Original Message----- From: Simon Glass [mailto:sjg@chromium.org] Sent: Monday, November 18, 2019 5:21 PM To: Vladimir Olovyannikov vladimir.olovyannikov@broadcom.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Heinrich Schuchardt xypron.glpk@gmx.de; Suji Velupiallai suji.velupillai@broadcom.com Subject: Re: [PATCH v1 1/1] cmd: adding malloc, math, and strcmp commands to u-boot
Hi Vladimir,
On Mon, 18 Nov 2019 at 16:27, Vladimir Olovyannikov vladimir.olovyannikov@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@broadcom.com Signed-off-by: Vladimir Olovyannikov
vladimir.olovyannikov@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 yhelpSupports allocation of heap memory and free allocated memorycommands.
These commands are used by u-boot scripts.+config CMD_MATH
bool "math"default yhelpProvides math commands such as add, sub, mul, div, shift,convert decimal to hex functionalities to be available in thescript.
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 yhelpProvides 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);elsereturn 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 inenvironment 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);elsereturn 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);elsereturn 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]);elsereturn 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