From e2d6a77a8fb0ea72eef5591648e90056ce4a75cd Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Thu, 14 Oct 2021 22:21:29 -0400 Subject: [PATCH 01/17] CI: Switch running the nokia_rx51 test with in-container toolchain Instead of fetching an arm toolchain to use, run the test with the one that's already in the container image. Signed-off-by: Tom Rini Reviewed-by: Simon Glass Tested-by: Simon Glass --- .azure-pipelines.yml | 3 +-- .gitlab-ci.yml | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 2ca146c0fd..b3794a956e 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -169,8 +169,7 @@ jobs: options: $(container_option) steps: - script: | - ./tools/buildman/buildman --fetch-arch arm - export PATH=~/.buildman-toolchains/gcc-9.2.0-nolibc/arm-linux-gnueabi/bin/:$PATH + export PATH=/opt/gcc-11.1.0-nolibc/arm-linux-gnueabi/bin:$PATH test/nokia_rx51_test.sh - job: test_py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 699ce991fb..e7c65ebbce 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -177,8 +177,7 @@ Run binman, buildman, dtoc, Kconfig and patman testsuites: Run tests for Nokia RX-51 (aka N900): stage: testsuites script: - - ./tools/buildman/buildman --fetch-arch arm; - export PATH=~/.buildman-toolchains/gcc-9.2.0-nolibc/arm-linux-gnueabi/bin/:$PATH; + - export PATH=/opt/gcc-11.1.0-nolibc/arm-linux-gnueabi/bin:$PATH; test/nokia_rx51_test.sh # Test sandbox with test.py From f7832ee552b6a54f2bcbde87ff205147930035b8 Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Tue, 5 Oct 2021 12:20:36 -0400 Subject: [PATCH 02/17] buildman: Add gcc-11.1.0 to the directory list While CI has been using gcc-11.1.0 for a long time, we have not updated buildman to match. Correct this omission. Signed-off-by: Tom Rini --- tools/buildman/toolchain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py index fd137f7300..4e2471f3e3 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -498,7 +498,7 @@ class Toolchains: if arch == 'aarch64': arch = 'arm64' base = 'https://www.kernel.org/pub/tools/crosstool/files/bin' - versions = ['9.2.0', '7.3.0', '6.4.0', '4.9.4'] + versions = ['11.1.0', '9.2.0', '7.3.0', '6.4.0', '4.9.4'] links = [] for version in versions: url = '%s/%s/%s/' % (base, arch, version) From 737fd142de530978157d61dd732273dd61a57eee Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 28 Aug 2021 11:42:09 +0200 Subject: [PATCH 03/17] sandbox: provide /chosen/boot-hartid property On RISC-V the sandbox must provide the /chosen/boot-hartid in the devicetree. Signed-off-by: Heinrich Schuchardt --- arch/sandbox/lib/Makefile | 2 +- arch/sandbox/lib/fdt_fixup.c | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 arch/sandbox/lib/fdt_fixup.c diff --git a/arch/sandbox/lib/Makefile b/arch/sandbox/lib/Makefile index b4ff717e78..a2bc5a7ee6 100644 --- a/arch/sandbox/lib/Makefile +++ b/arch/sandbox/lib/Makefile @@ -5,7 +5,7 @@ # (C) Copyright 2002-2006 # Wolfgang Denk, DENX Software Engineering, wd@denx.de. -obj-y += interrupts.o sections.o +obj-y += fdt_fixup.o interrupts.o sections.o obj-$(CONFIG_PCI) += pci_io.o obj-$(CONFIG_CMD_BOOTM) += bootm.o obj-$(CONFIG_CMD_BOOTZ) += bootm.o diff --git a/arch/sandbox/lib/fdt_fixup.c b/arch/sandbox/lib/fdt_fixup.c new file mode 100644 index 0000000000..a646f2059c --- /dev/null +++ b/arch/sandbox/lib/fdt_fixup.c @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#define LOG_CATEGORY LOGC_ARCH + +#include +#include +#include + +#if defined(__riscv) +int arch_fixup_fdt(void *blob) +{ + int ret; + + ret = fdt_find_or_add_subnode(blob, 0, "chosen");; + if (ret < 0) + goto err; + ret = fdt_setprop_u32(blob, ret, "boot-hartid", 1); + if (ret < 0) + goto err; + return 0; +err: + log_err("Setting /chosen/boot-hartid failed: %s\n", fdt_strerror(ret)); + return ret; +} +#endif From 3610970872351b58d9cfdbd9bf9aeabc6adac7b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Thu, 30 Sep 2021 17:52:45 +0200 Subject: [PATCH 04/17] dm: Fix util.h's broken include guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix up the header's include guard to contain the definition of dm_priv_to_rw(), which was erroneously added outside of it, by moving its #endif to the end of the file (i.e. where it belongs). This removes the risk of compilation errors resulting from the redefinition of that function where the header might have been (indirectly) included more than once. Fixes: cfb9c9b77c2 ("dm: core: Use separate priv/plat data region") Signed-off-by: Pierre-Clément Tosi Cc: Simon Glass --- include/dm/util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/dm/util.h b/include/dm/util.h index c634e470e7..17baf55c25 100644 --- a/include/dm/util.h +++ b/include/dm/util.h @@ -48,8 +48,6 @@ void dm_dump_driver_compat(void); /* Dump out a list of drivers with static platform data */ void dm_dump_static_driver_info(void); -#endif - #if CONFIG_IS_ENABLED(OF_PLATDATA_INST) && CONFIG_IS_ENABLED(READ_ONLY) void *dm_priv_to_rw(void *priv); #else @@ -58,3 +56,5 @@ static inline void *dm_priv_to_rw(void *priv) return priv; } #endif + +#endif From 6dc1e2f10c7ca8ba7caf5444efeb5bb015837829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:26 +0200 Subject: [PATCH 05/17] env: Fix documentation for env_get_f() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function actually returns: - the number of bytes written into @buf excluding the terminating NULL-byte, if there was enough space in @buf - the number of bytes written into @buf including the terminating NULL-byte, if there wasn't enough space in @buf - -1 if the variable is not found Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- include/env.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/env.h b/include/env.h index d5e2bcb530..b1a4003681 100644 --- a/include/env.h +++ b/include/env.h @@ -131,7 +131,10 @@ char *from_env(const char *envvar); * support reading the value (slowly) and some will not. * * @varname: Variable to look up - * @return value of variable, or NULL if not found + * @return number of bytes written into @buf, excluding the terminating + * NULL-byte if there was enough space in @buf, and including the + * terminating NULL-byte if there wasn't enough space, or -1 if the + * variable is not found */ int env_get_f(const char *name, char *buf, unsigned int len); From dd1c5a7f80baf69ac94122acb3202bdcf14fec92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:27 +0200 Subject: [PATCH 06/17] env: Drop env_get_char_spec() and old, unused .get_char() implementations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit b2cdef4861be ("env: restore old env_get_char() behaviour") dropped the .get_char() method from struct env_driver, but left the two existing implementations (eeprom and nvram) in case someone would use them by overwriting weak function env_get_char_spec(). Since this was never done in the 3.5 years, let's drop these methods and simplify the code. Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- env/eeprom.c | 18 ------------------ env/env.c | 7 +------ env/nvram.c | 14 -------------- 3 files changed, 1 insertion(+), 38 deletions(-) diff --git a/env/eeprom.c b/env/eeprom.c index 253bdf1428..f8556a4721 100644 --- a/env/eeprom.c +++ b/env/eeprom.c @@ -64,24 +64,6 @@ static int eeprom_bus_write(unsigned dev_addr, unsigned offset, return rcode; } -/** Call this function from overridden env_get_char_spec() if you need - * this functionality. - */ -int env_eeprom_get_char(int index) -{ - uchar c; - unsigned int off = CONFIG_ENV_OFFSET; - -#ifdef CONFIG_ENV_OFFSET_REDUND - if (gd->env_valid == ENV_REDUND) - off = CONFIG_ENV_OFFSET_REDUND; -#endif - eeprom_bus_read(CONFIG_SYS_I2C_EEPROM_ADDR, - off + index + offsetof(env_t, data), &c, 1); - - return c; -} - static int env_eeprom_load(void) { char buf_env[CONFIG_ENV_SIZE]; diff --git a/env/env.c b/env/env.c index e534008006..91d220c3dd 100644 --- a/env/env.c +++ b/env/env.c @@ -166,17 +166,12 @@ static struct env_driver *env_driver_lookup(enum env_operation op, int prio) return drv; } -__weak int env_get_char_spec(int index) -{ - return *(uchar *)(gd->env_addr + index); -} - int env_get_char(int index) { if (gd->env_valid == ENV_INVALID) return default_environment[index]; else - return env_get_char_spec(index); + return *(uchar *)(gd->env_addr + index); } int env_load(void) diff --git a/env/nvram.c b/env/nvram.c index f4126858b5..261b31edfb 100644 --- a/env/nvram.c +++ b/env/nvram.c @@ -42,20 +42,6 @@ extern void nvram_write(long dest, const void *src, size_t count); static env_t *env_ptr = (env_t *)CONFIG_ENV_ADDR; #endif -#ifdef CONFIG_SYS_NVRAM_ACCESS_ROUTINE -/** Call this function from overridden env_get_char_spec() if you need - * this functionality. - */ -int env_nvram_get_char(int index) -{ - uchar c; - - nvram_read(&c, CONFIG_ENV_ADDR + index, 1); - - return c; -} -#endif - static int env_nvram_load(void) { char buf[CONFIG_ENV_SIZE]; From 6aa652d008398fa7db99b7d570bc6f0123dcd9b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:28 +0200 Subject: [PATCH 07/17] examples: api: glue: Remove comment that does not apply anymore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This comment is not true since commit 6215bd4c1fd6 ("api: Use hashtable function for API_env_enum"). Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- examples/api/glue.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/examples/api/glue.c b/examples/api/glue.c index 91d13157a1..075d307ae2 100644 --- a/examples/api/glue.c +++ b/examples/api/glue.c @@ -365,11 +365,6 @@ const char * ub_env_enum(const char *last) env = NULL; - /* - * It's OK to pass only the name piece as last (and not the whole - * 'name=val' string), since the API_ENUM_ENV call uses env_match() - * internally, which handles such case - */ if (!syscall(API_ENV_ENUM, NULL, last, &env)) return NULL; From 7b611ee90e1e4db531c4e3896efebfdc0743725d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:29 +0200 Subject: [PATCH 08/17] env: Change env_match() to static and remove from header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function was used by other parts of U-Boot in the past when environment was read from underlying device one character at a time. This is not the case anymore. Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 30 +++++++++++++++--------------- include/env.h | 11 ----------- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index ddc715b4f9..742e0924af 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -706,6 +706,21 @@ char *from_env(const char *envvar) return ret; } +static int env_match(uchar *s1, int i2) +{ + if (s1 == NULL) + return -1; + + while (*s1 == env_get_char(i2++)) + if (*s1++ == '=') + return i2; + + if (*s1 == '\0' && env_get_char(i2-1) == '=') + return i2; + + return -1; +} + /* * Look up variable from environment for restricted C runtime env. */ @@ -816,21 +831,6 @@ static int do_env_select(struct cmd_tbl *cmdtp, int flag, int argc, #endif /* CONFIG_SPL_BUILD */ -int env_match(uchar *s1, int i2) -{ - if (s1 == NULL) - return -1; - - while (*s1 == env_get_char(i2++)) - if (*s1++ == '=') - return i2; - - if (*s1 == '\0' && env_get_char(i2-1) == '=') - return i2; - - return -1; -} - #ifndef CONFIG_SPL_BUILD static int do_env_default(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) diff --git a/include/env.h b/include/env.h index b1a4003681..a9b2a4c8b2 100644 --- a/include/env.h +++ b/include/env.h @@ -90,17 +90,6 @@ int env_init(void); */ void env_relocate(void); -/** - * env_match() - Match a name / name=value pair - * - * This is used prior to relocation for finding envrionment variables - * - * @name: A simple 'name', or a 'name=value' pair. - * @index: The environment index for a 'name2=value2' pair. - * @return index for the value if the names match, else -1. - */ -int env_match(unsigned char *name, int index); - /** * env_get() - Look up the value of an environment variable * From 52f9ed34cb3768663ee9c2e5d980143e8ac783a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:30 +0200 Subject: [PATCH 09/17] env: Inline env_get_char() into its only user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function is a relic from the past when environment was read from underlying device one character at a time. It is used only in the case when getting an environemnt variable prior relocation, and the function is simple enough to be inlined there. Since env_get_char() is being changed to simple access to an array, we can drop the failing cases and simplify the code (this could have been done before, since env_get_char() did not fail even before). Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 28 ++++++++++++++-------------- env/env.c | 8 -------- env/nowhere.c | 5 ++--- include/env.h | 10 ---------- 4 files changed, 16 insertions(+), 35 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 742e0924af..3952deda1b 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -706,16 +706,16 @@ char *from_env(const char *envvar) return ret; } -static int env_match(uchar *s1, int i2) +static int env_match(const char *env, const char *s1, int i2) { if (s1 == NULL) return -1; - while (*s1 == env_get_char(i2++)) + while (*s1 == env[i2++]) if (*s1++ == '=') return i2; - if (*s1 == '\0' && env_get_char(i2-1) == '=') + if (*s1 == '\0' && env[i2-1] == '=') return i2; return -1; @@ -726,28 +726,28 @@ static int env_match(uchar *s1, int i2) */ int env_get_f(const char *name, char *buf, unsigned len) { - int i, nxt, c; + const char *env; + int i, nxt; - for (i = 0; env_get_char(i) != '\0'; i = nxt + 1) { + if (gd->env_valid == ENV_INVALID) + env = (const char *)default_environment; + else + env = (const char *)gd->env_addr; + + for (i = 0; env[i] != '\0'; i = nxt + 1) { int val, n; - for (nxt = i; (c = env_get_char(nxt)) != '\0'; ++nxt) { - if (c < 0) - return c; + for (nxt = i; env[nxt] != '\0'; ++nxt) if (nxt >= CONFIG_ENV_SIZE) return -1; - } - val = env_match((uchar *)name, i); + val = env_match(env, name, i); if (val < 0) continue; /* found; copy out */ for (n = 0; n < len; ++n, ++buf) { - c = env_get_char(val++); - if (c < 0) - return c; - *buf = c; + *buf = env[val++]; if (*buf == '\0') return n; } diff --git a/env/env.c b/env/env.c index 91d220c3dd..e4dfb92e15 100644 --- a/env/env.c +++ b/env/env.c @@ -166,14 +166,6 @@ static struct env_driver *env_driver_lookup(enum env_operation op, int prio) return drv; } -int env_get_char(int index) -{ - if (gd->env_valid == ENV_INVALID) - return default_environment[index]; - else - return *(uchar *)(gd->env_addr + index); -} - int env_load(void) { struct env_driver *drv; diff --git a/env/nowhere.c b/env/nowhere.c index 41557f5ce4..1fcf503453 100644 --- a/env/nowhere.c +++ b/env/nowhere.c @@ -31,9 +31,8 @@ static int env_nowhere_init(void) static int env_nowhere_load(void) { /* - * for SPL, set env_valid = ENV_INVALID is enough as env_get_char() - * return the default env if env_get is used - * and SPL don't used env_import to reduce its size + * For SPL, setting env_valid = ENV_INVALID is enough, as env_get() + * searches default_environment array in that case. * For U-Boot proper, import the default environment to allow reload. */ if (!IS_ENABLED(CONFIG_SPL_BUILD)) diff --git a/include/env.h b/include/env.h index a9b2a4c8b2..220ab979d9 100644 --- a/include/env.h +++ b/include/env.h @@ -351,16 +351,6 @@ char *env_get_default(const char *name); /* [re]set to the default environment */ void env_set_default(const char *s, int flags); -/** - * env_get_char() - Get a character from the early environment - * - * This reads from the pre-relocation environment - * - * @index: Index of character to read (0 = first) - * @return character read, or -ve on error - */ -int env_get_char(int index); - /** * env_reloc() - Relocate the 'env' sub-commands * From eff73b2eed28389805e8dda6632f6c5f29b3e9b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:31 +0200 Subject: [PATCH 10/17] env: Use string pointer instead of indexes in env_get_f() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we no longer use env_get_char() to access n-th character of linearized environment data, but rather access the arrays themselves, we can convert the iteration to use string pointers instead of position indexes. Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 3952deda1b..f395d2893a 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -706,19 +706,19 @@ char *from_env(const char *envvar) return ret; } -static int env_match(const char *env, const char *s1, int i2) +static const char *env_match(const char *p, const char *s1) { if (s1 == NULL) - return -1; + return NULL; - while (*s1 == env[i2++]) + while (*s1 == *p++) if (*s1++ == '=') - return i2; + return p; - if (*s1 == '\0' && env[i2-1] == '=') - return i2; + if (*s1 == '\0' && p[-1] == '=') + return p; - return -1; + return NULL; } /* @@ -726,28 +726,28 @@ static int env_match(const char *env, const char *s1, int i2) */ int env_get_f(const char *name, char *buf, unsigned len) { - const char *env; - int i, nxt; + const char *env, *p, *nxt; if (gd->env_valid == ENV_INVALID) env = (const char *)default_environment; else env = (const char *)gd->env_addr; - for (i = 0; env[i] != '\0'; i = nxt + 1) { - int val, n; + for (p = env; *p != '\0'; p = nxt + 1) { + const char *value; + int n; - for (nxt = i; env[nxt] != '\0'; ++nxt) - if (nxt >= CONFIG_ENV_SIZE) + for (nxt = p; *nxt != '\0'; ++nxt) + if (nxt - env >= CONFIG_ENV_SIZE) return -1; - val = env_match(env, name, i); - if (val < 0) + value = env_match(p, name); + if (value == NULL) continue; /* found; copy out */ for (n = 0; n < len; ++n, ++buf) { - *buf = env[val++]; + *buf = *value++; if (*buf == '\0') return n; } From d1bca8d2a18eb4a850c0ecbc9327eb3dfa5df5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:32 +0200 Subject: [PATCH 11/17] env: Use better name for variable in env_get_f() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `nxt` variable actually points to the terminating null-byte of the current env var, and the next env var is at `nxt + 1`, not `nxt`. So a better name for this variable is `end`. Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index f395d2893a..5b1d4c2448 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -726,19 +726,19 @@ static const char *env_match(const char *p, const char *s1) */ int env_get_f(const char *name, char *buf, unsigned len) { - const char *env, *p, *nxt; + const char *env, *p, *end; if (gd->env_valid == ENV_INVALID) env = (const char *)default_environment; else env = (const char *)gd->env_addr; - for (p = env; *p != '\0'; p = nxt + 1) { + for (p = env; *p != '\0'; p = end + 1) { const char *value; int n; - for (nxt = p; *nxt != '\0'; ++nxt) - if (nxt - env >= CONFIG_ENV_SIZE) + for (end = p; *end != '\0'; ++end) + if (end - env >= CONFIG_ENV_SIZE) return -1; value = env_match(p, name); From 6ba4473fb757e7606a4224d87774294c9d60d2cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:33 +0200 Subject: [PATCH 12/17] env: Don't match empty variable name in env_match() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do we really allow zero-length variable name? I guess not. Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 5b1d4c2448..8d53579d92 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -708,7 +708,7 @@ char *from_env(const char *envvar) static const char *env_match(const char *p, const char *s1) { - if (s1 == NULL) + if (s1 == NULL || *s1 == '\0') return NULL; while (*s1 == *p++) From 6b6e3eeba9f96b5d908a9b46b7bb25e76109a0c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:34 +0200 Subject: [PATCH 13/17] env: Early return from env_get_f() on NULL name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test non-NULL name immediately, not in env_match(). Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 8d53579d92..063cc76282 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -708,9 +708,6 @@ char *from_env(const char *envvar) static const char *env_match(const char *p, const char *s1) { - if (s1 == NULL || *s1 == '\0') - return NULL; - while (*s1 == *p++) if (*s1++ == '=') return p; @@ -728,6 +725,9 @@ int env_get_f(const char *name, char *buf, unsigned len) { const char *env, *p, *end; + if (name == NULL || *name == '\0') + return -1; + if (gd->env_valid == ENV_INVALID) env = (const char *)default_environment; else From 3112ce0ce8195880aec5e9373434a85e21c3e1af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:35 +0200 Subject: [PATCH 14/17] env: Make return value of env_get_f() behave like sprintf() on success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the env_get_f() function's return value behaves weirdly: it returns the number of bytes written into `buf`, but whether this is excluding the terminating NULL-byte or including it depends on whether there was enough space in `buf`. Change the function to always return the actual length of the value of the environment variable (excluding the terminating NULL-byte) on success. This makes it behave like sprintf(). All users of this function in U-Boot are compatible with this change. Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 8 +++++--- include/env.h | 6 ++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 063cc76282..527b522e9e 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -735,7 +735,7 @@ int env_get_f(const char *name, char *buf, unsigned len) for (p = env; *p != '\0'; p = end + 1) { const char *value; - int n; + int n, res; for (end = p; *end != '\0'; ++end) if (end - env >= CONFIG_ENV_SIZE) @@ -745,11 +745,13 @@ int env_get_f(const char *name, char *buf, unsigned len) if (value == NULL) continue; + res = end - value; + /* found; copy out */ for (n = 0; n < len; ++n, ++buf) { *buf = *value++; if (*buf == '\0') - return n; + return res; } if (n) @@ -758,7 +760,7 @@ int env_get_f(const char *name, char *buf, unsigned len) printf("env_buf [%u bytes] too small for value of \"%s\"\n", len, name); - return n; + return res; } return -1; diff --git a/include/env.h b/include/env.h index 220ab979d9..ee5e30d036 100644 --- a/include/env.h +++ b/include/env.h @@ -120,10 +120,8 @@ char *from_env(const char *envvar); * support reading the value (slowly) and some will not. * * @varname: Variable to look up - * @return number of bytes written into @buf, excluding the terminating - * NULL-byte if there was enough space in @buf, and including the - * terminating NULL-byte if there wasn't enough space, or -1 if the - * variable is not found + * @return actual length of the variable value excluding the terminating + * NULL-byte, or -1 if the variable is not found */ int env_get_f(const char *name, char *buf, unsigned int len); From a473766cce44882237e567bcd58f7559ecdd0440 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:36 +0200 Subject: [PATCH 15/17] env: Use memcpy() instead of ad-hoc code to copy variable value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copy the value of the found variable into given buffer with memcpy() instead of ad-hoc code. Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 527b522e9e..ffcfb55f10 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -735,7 +735,7 @@ int env_get_f(const char *name, char *buf, unsigned len) for (p = env; *p != '\0'; p = end + 1) { const char *value; - int n, res; + unsigned res; for (end = p; *end != '\0'; ++end) if (end - env >= CONFIG_ENV_SIZE) @@ -746,20 +746,14 @@ int env_get_f(const char *name, char *buf, unsigned len) continue; res = end - value; + memcpy(buf, value, min(len, res + 1)); - /* found; copy out */ - for (n = 0; n < len; ++n, ++buf) { - *buf = *value++; - if (*buf == '\0') - return res; + if (len <= res) { + buf[len - 1] = '\0'; + printf("env_buf [%u bytes] too small for value of \"%s\"\n", + len, name); } - if (n) - *--buf = '\0'; - - printf("env_buf [%u bytes] too small for value of \"%s\"\n", - len, name); - return res; } From a80652ebb394abfd508feb38090228f5e5290d32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:37 +0200 Subject: [PATCH 16/17] env: Simplify env_match() and inline into env_get_f() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the past the env_match() function was used to match envs with - name, i.e. string "name" - variable assignment, i.e. string "name=other_value" The latter is not the case anymore, since the env_match() function is now used only in env_get_f(), and so we can simplify the function into a simple strncmp() with an additional comparison to '='. Let's do this, and since the resulting function is quite simple, let's also inline its code into env_get_f(). Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index ffcfb55f10..272d0c7718 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -706,28 +706,19 @@ char *from_env(const char *envvar) return ret; } -static const char *env_match(const char *p, const char *s1) -{ - while (*s1 == *p++) - if (*s1++ == '=') - return p; - - if (*s1 == '\0' && p[-1] == '=') - return p; - - return NULL; -} - /* * Look up variable from environment for restricted C runtime env. */ int env_get_f(const char *name, char *buf, unsigned len) { const char *env, *p, *end; + size_t name_len; if (name == NULL || *name == '\0') return -1; + name_len = strlen(name); + if (gd->env_valid == ENV_INVALID) env = (const char *)default_environment; else @@ -741,9 +732,9 @@ int env_get_f(const char *name, char *buf, unsigned len) if (end - env >= CONFIG_ENV_SIZE) return -1; - value = env_match(p, name); - if (value == NULL) + if (strncmp(name, p, name_len) || p[name_len] != '=') continue; + value = &p[name_len + 1]; res = end - value; memcpy(buf, value, min(len, res + 1)); From f231566475c545de476a3bf5f596c246c52511aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 17 Oct 2021 17:36:38 +0200 Subject: [PATCH 17/17] env: Move non-cli env functions to env/common.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the following functions from cmd/nvedit.c to env/common.c: env_set_ulong() env_set_hex() env_get_hex() eth_env_get_enetaddr() eth_env_set_enetaddr() env_get() from_env() env_get_f() env_get_ulong() since these functions are not specific for U-Boot's CLI. We leave env_set() in cmd/nvedit.c, since it calls _do_env_set(), which is a static function in that file. Signed-off-by: Marek Behún Reviewed-by: Simon Glass --- cmd/nvedit.c | 175 ------------------------------------------------- env/common.c | 180 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 175 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 272d0c7718..3bb6e764c0 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -38,7 +37,6 @@ #include #include #include -#include #include #include #include @@ -320,69 +318,6 @@ int env_set(const char *varname, const char *varvalue) return _do_env_set(0, 3, (char * const *)argv, H_PROGRAMMATIC); } -/** - * Set an environment variable to an integer value - * - * @param varname Environment variable to set - * @param value Value to set it to - * @return 0 if ok, 1 on error - */ -int env_set_ulong(const char *varname, ulong value) -{ - /* TODO: this should be unsigned */ - char *str = simple_itoa(value); - - return env_set(varname, str); -} - -/** - * Set an environment variable to an value in hex - * - * @param varname Environment variable to set - * @param value Value to set it to - * @return 0 if ok, 1 on error - */ -int env_set_hex(const char *varname, ulong value) -{ - char str[17]; - - sprintf(str, "%lx", value); - return env_set(varname, str); -} - -ulong env_get_hex(const char *varname, ulong default_val) -{ - const char *s; - ulong value; - char *endp; - - s = env_get(varname); - if (s) - value = hextoul(s, &endp); - if (!s || endp == s) - return default_val; - - return value; -} - -int eth_env_get_enetaddr(const char *name, uint8_t *enetaddr) -{ - string_to_enetaddr(env_get(name), enetaddr); - return is_valid_ethaddr(enetaddr); -} - -int eth_env_set_enetaddr(const char *name, const uint8_t *enetaddr) -{ - char buf[ARP_HLEN_ASCII + 1]; - - if (eth_env_get_enetaddr(name, (uint8_t *)buf)) - return -EEXIST; - - sprintf(buf, "%pM", enetaddr); - - return env_set(name, buf); -} - #ifndef CONFIG_SPL_BUILD static int do_env_set(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -661,117 +596,7 @@ static int do_env_edit(struct cmd_tbl *cmdtp, int flag, int argc, } } #endif /* CONFIG_CMD_EDITENV */ -#endif /* CONFIG_SPL_BUILD */ -/* - * Look up variable from environment, - * return address of storage for that variable, - * or NULL if not found - */ -char *env_get(const char *name) -{ - if (gd->flags & GD_FLG_ENV_READY) { /* after import into hashtable */ - struct env_entry e, *ep; - - WATCHDOG_RESET(); - - e.key = name; - e.data = NULL; - hsearch_r(e, ENV_FIND, &ep, &env_htab, 0); - - return ep ? ep->data : NULL; - } - - /* restricted capabilities before import */ - if (env_get_f(name, (char *)(gd->env_buf), sizeof(gd->env_buf)) > 0) - return (char *)(gd->env_buf); - - return NULL; -} - -/* - * Like env_get, but prints an error if envvar isn't defined in the - * environment. It always returns what env_get does, so it can be used in - * place of env_get without changing error handling otherwise. - */ -char *from_env(const char *envvar) -{ - char *ret; - - ret = env_get(envvar); - - if (!ret) - printf("missing environment variable: %s\n", envvar); - - return ret; -} - -/* - * Look up variable from environment for restricted C runtime env. - */ -int env_get_f(const char *name, char *buf, unsigned len) -{ - const char *env, *p, *end; - size_t name_len; - - if (name == NULL || *name == '\0') - return -1; - - name_len = strlen(name); - - if (gd->env_valid == ENV_INVALID) - env = (const char *)default_environment; - else - env = (const char *)gd->env_addr; - - for (p = env; *p != '\0'; p = end + 1) { - const char *value; - unsigned res; - - for (end = p; *end != '\0'; ++end) - if (end - env >= CONFIG_ENV_SIZE) - return -1; - - if (strncmp(name, p, name_len) || p[name_len] != '=') - continue; - value = &p[name_len + 1]; - - res = end - value; - memcpy(buf, value, min(len, res + 1)); - - if (len <= res) { - buf[len - 1] = '\0'; - printf("env_buf [%u bytes] too small for value of \"%s\"\n", - len, name); - } - - return res; - } - - return -1; -} - -/** - * Decode the integer value of an environment variable and return it. - * - * @param name Name of environment variable - * @param base Number base to use (normally 10, or 16 for hex) - * @param default_val Default value to return if the variable is not - * found - * @return the decoded value, or default_val if not found - */ -ulong env_get_ulong(const char *name, int base, ulong default_val) -{ - /* - * We can use env_get() here, even before relocation, since the - * environment variable value is an integer and thus short. - */ - const char *str = env_get(name); - - return str ? simple_strtoul(str, NULL, base) : default_val; -} - -#ifndef CONFIG_SPL_BUILD #if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) static int do_env_save(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) diff --git a/env/common.c b/env/common.c index 81e9e0b2aa..db213b7748 100644 --- a/env/common.c +++ b/env/common.c @@ -21,6 +21,8 @@ #include #include #include +#include +#include DECLARE_GLOBAL_DATA_PTR; @@ -33,6 +35,184 @@ struct hsearch_data env_htab = { .change_ok = env_flags_validate, }; +/* + * This env_set() function is defined in cmd/nvedit.c, since it calls + * _do_env_set(), whis is a static function in that file. + * + * int env_set(const char *varname, const char *varvalue); + */ + +/** + * Set an environment variable to an integer value + * + * @param varname Environment variable to set + * @param value Value to set it to + * @return 0 if ok, 1 on error + */ +int env_set_ulong(const char *varname, ulong value) +{ + /* TODO: this should be unsigned */ + char *str = simple_itoa(value); + + return env_set(varname, str); +} + +/** + * Set an environment variable to an value in hex + * + * @param varname Environment variable to set + * @param value Value to set it to + * @return 0 if ok, 1 on error + */ +int env_set_hex(const char *varname, ulong value) +{ + char str[17]; + + sprintf(str, "%lx", value); + return env_set(varname, str); +} + +ulong env_get_hex(const char *varname, ulong default_val) +{ + const char *s; + ulong value; + char *endp; + + s = env_get(varname); + if (s) + value = hextoul(s, &endp); + if (!s || endp == s) + return default_val; + + return value; +} + +int eth_env_get_enetaddr(const char *name, uint8_t *enetaddr) +{ + string_to_enetaddr(env_get(name), enetaddr); + return is_valid_ethaddr(enetaddr); +} + +int eth_env_set_enetaddr(const char *name, const uint8_t *enetaddr) +{ + char buf[ARP_HLEN_ASCII + 1]; + + if (eth_env_get_enetaddr(name, (uint8_t *)buf)) + return -EEXIST; + + sprintf(buf, "%pM", enetaddr); + + return env_set(name, buf); +} + +/* + * Look up variable from environment, + * return address of storage for that variable, + * or NULL if not found + */ +char *env_get(const char *name) +{ + if (gd->flags & GD_FLG_ENV_READY) { /* after import into hashtable */ + struct env_entry e, *ep; + + WATCHDOG_RESET(); + + e.key = name; + e.data = NULL; + hsearch_r(e, ENV_FIND, &ep, &env_htab, 0); + + return ep ? ep->data : NULL; + } + + /* restricted capabilities before import */ + if (env_get_f(name, (char *)(gd->env_buf), sizeof(gd->env_buf)) > 0) + return (char *)(gd->env_buf); + + return NULL; +} + +/* + * Like env_get, but prints an error if envvar isn't defined in the + * environment. It always returns what env_get does, so it can be used in + * place of env_get without changing error handling otherwise. + */ +char *from_env(const char *envvar) +{ + char *ret; + + ret = env_get(envvar); + + if (!ret) + printf("missing environment variable: %s\n", envvar); + + return ret; +} + +/* + * Look up variable from environment for restricted C runtime env. + */ +int env_get_f(const char *name, char *buf, unsigned len) +{ + const char *env, *p, *end; + size_t name_len; + + if (name == NULL || *name == '\0') + return -1; + + name_len = strlen(name); + + if (gd->env_valid == ENV_INVALID) + env = (const char *)default_environment; + else + env = (const char *)gd->env_addr; + + for (p = env; *p != '\0'; p = end + 1) { + const char *value; + unsigned res; + + for (end = p; *end != '\0'; ++end) + if (end - env >= CONFIG_ENV_SIZE) + return -1; + + if (strncmp(name, p, name_len) || p[name_len] != '=') + continue; + value = &p[name_len + 1]; + + res = end - value; + memcpy(buf, value, min(len, res + 1)); + + if (len <= res) { + buf[len - 1] = '\0'; + printf("env_buf [%u bytes] too small for value of \"%s\"\n", + len, name); + } + + return res; + } + + return -1; +} + +/** + * Decode the integer value of an environment variable and return it. + * + * @param name Name of environment variable + * @param base Number base to use (normally 10, or 16 for hex) + * @param default_val Default value to return if the variable is not + * found + * @return the decoded value, or default_val if not found + */ +ulong env_get_ulong(const char *name, int base, ulong default_val) +{ + /* + * We can use env_get() here, even before relocation, since the + * environment variable value is an integer and thus short. + */ + const char *str = env_get(name); + + return str ? simple_strtoul(str, NULL, base) : default_val; +} + /* * Read an environment variable as a boolean * Return -1 if variable does not exist (default to true)