[PATCH 00/10] env_get_char() removal and env_get_f() refactor
From: Marek Behún marek.behun@nic.cz
Hi Simon, Tom,
env_get_char() is a relic from the past when env was read char-by-char from underlying device. Currently it only accesses in-memory arrays. We can remove it and access the arrays directly. This simplifies the old code of env_get_f().
Marek
Marek Behún (10): env: Drop env_get_char_spec() and old, unused .get_char() implementations examples: api: glue: Remove comment that does not apply anymore env: Change env_match() to static and remove from header env: Don't match empty variable name in env_match() env: Check for terminating null-byte in env_match() env: Inline env_get_char() into it's only user env: Early return from env_get_f() on NULL name env: Use strncpy() instead of ad-hoc code to copy variable value env: Use string pointer instead of indexes in env_get_f() env: Move non-cmd specific env functions to env/common.c
cmd/nvedit.c | 188 -------------------------------------------- env/common.c | 180 ++++++++++++++++++++++++++++++++++++++++++ env/eeprom.c | 18 ----- env/env.c | 13 --- env/nowhere.c | 5 +- env/nvram.c | 14 ---- examples/api/glue.c | 5 -- include/env.h | 21 ----- 8 files changed, 182 insertions(+), 262 deletions(-)
From: Marek Behún marek.behun@nic.cz
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 marek.behun@nic.cz --- 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: Marek Behún marek.behun@nic.cz
This comment is not true since commit 6215bd4c1fd6 ("api: Use hashtable function for API_env_enum").
Signed-off-by: Marek Behún marek.behun@nic.cz --- 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: Marek Behún marek.behun@nic.cz
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 marek.behun@nic.cz --- 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 d5e2bcb530..6b774ae078 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: Marek Behún marek.behun@nic.cz
Do we really allow zero-length variable name? I guess not.
Signed-off-by: Marek Behún marek.behun@nic.cz --- cmd/nvedit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 742e0924af..e2e8a38b5d 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -708,7 +708,7 @@ char *from_env(const char *envvar)
static int env_match(uchar *s1, int i2) { - if (s1 == NULL) + if (s1 == NULL || *s1 == '\0') return -1;
while (*s1 == env_get_char(i2++))
From: Marek Behún marek.behun@nic.cz
There is a possible overflow in env_match(): if environment contains a terminating null-byte before '=' character (i.e. environment is broken), the env_match() function can access data after the terminating null-byte from parameter pointer.
Example: if env_get_char() returns characters from string array "abc\0def\0" and env_match("abc", 0) is called, the function will access at least one byte after the end of the "abc" literal.
Signed-off-by: Marek Behún marek.behun@nic.cz --- cmd/nvedit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index e2e8a38b5d..a516491832 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -711,7 +711,7 @@ static int env_match(uchar *s1, int i2) if (s1 == NULL || *s1 == '\0') return -1;
- while (*s1 == env_get_char(i2++)) + while (*s1 != '\0' && *s1 == env_get_char(i2++)) if (*s1++ == '=') return i2;
On Tue, 12 Oct 2021 13:04:56 +0200 Marek Behún kabel@kernel.org wrote:
- while (*s1 == env_get_char(i2++))
- while (*s1 != '\0' && *s1 == env_get_char(i2++))
This check has to be done in the other order: while (*s1 == env_get_char(i2++) && *s1 != '\0')
so that i2 gets incremented even if *s1 == '\0'.
Will be fixed in v2.
From: Marek Behún marek.behun@nic.cz
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 marek.behun@nic.cz --- 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 a516491832..4664b7b872 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 || *s1 == '\0') return -1;
- while (*s1 != '\0' && *s1 == env_get_char(i2++)) + while (*s1 != '\0' && *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 6b774ae078..412b2bf21b 100644 --- a/include/env.h +++ b/include/env.h @@ -348,16 +348,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: Marek Behún marek.behun@nic.cz
Test non-NULL name immediately, not in env_match().
Signed-off-by: Marek Behún marek.behun@nic.cz --- cmd/nvedit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 4664b7b872..412918a000 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -708,9 +708,6 @@ char *from_env(const char *envvar)
static int env_match(const char *env, const char *s1, int i2) { - if (s1 == NULL || *s1 == '\0') - return -1; - while (*s1 != '\0' && *s1 == env[i2++]) if (*s1++ == '=') return i2; @@ -729,6 +726,9 @@ int env_get_f(const char *name, char *buf, unsigned len) const char *env; int i, nxt;
+ if (name == NULL || *name == '\0') + return -1; + if (gd->env_valid == ENV_INVALID) env = (const char *)default_environment; else
From: Marek Behún marek.behun@nic.cz
Copy the value of the found variable into given buffer with strncpy().
Signed-off-by: Marek Behún marek.behun@nic.cz --- cmd/nvedit.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 412918a000..51b9e4ffa4 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -746,18 +746,13 @@ int env_get_f(const char *name, char *buf, unsigned len) continue;
/* found; copy out */ - for (n = 0; n < len; ++n, ++buf) { - *buf = env[val++]; - if (*buf == '\0') - return n; + n = strncpy(buf, &env[val], len) - buf; + if (len && n == len) { + 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 n; }
On 12/10/2021 13.04, Marek Behún wrote:
From: Marek Behún marek.behun@nic.cz
Copy the value of the found variable into given buffer with strncpy().
Signed-off-by: Marek Behún marek.behun@nic.cz
cmd/nvedit.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 412918a000..51b9e4ffa4 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -746,18 +746,13 @@ int env_get_f(const char *name, char *buf, unsigned len) continue;
/* found; copy out */
for (n = 0; n < len; ++n, ++buf) {*buf = env[val++];if (*buf == '\0')return n;
n = strncpy(buf, &env[val], len) - buf;
strncpy by definition returns the dst argument, so this is, apart from the side effects done by strncpy, a complicated way of saying "n = 0;".
if (len && n == len) {
which then makes this automatically false always.
I understand why you want to avoid an open-coded copying, but strncpy is almost certainly the wrong tool for the job. Apart from not revealing anything about the actual length of the source string, it also has the well-known defect of zeroing the tail of the buffer in the (usual) case where the source is shorter than the buffer.
n = strlcpy(buf, &env[val], len); if (n >= len) ...
is probably closer to what you want. But only if you can trust &env[val] to actually have a nul byte before going into lala land.
Rasmus
On Tue, 12 Oct 2021 13:41:43 +0200 Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 12/10/2021 13.04, Marek Behún wrote:
From: Marek Behún marek.behun@nic.cz
Copy the value of the found variable into given buffer with strncpy().
Signed-off-by: Marek Behún marek.behun@nic.cz
cmd/nvedit.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 412918a000..51b9e4ffa4 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -746,18 +746,13 @@ int env_get_f(const char *name, char *buf, unsigned len) continue;
/* found; copy out */
for (n = 0; n < len; ++n, ++buf) {*buf = env[val++];if (*buf == '\0')return n;
n = strncpy(buf, &env[val], len) - buf;strncpy by definition returns the dst argument, so this is, apart from the side effects done by strncpy, a complicated way of saying "n = 0;".
You are right, this is a mistake.
if (len && n == len) {which then makes this automatically false always.
I understand why you want to avoid an open-coded copying, but strncpy is almost certainly the wrong tool for the job. Apart from not revealing anything about the actual length of the source string, it also has the well-known defect of zeroing the tail of the buffer in the (usual) case where the source is shorter than the buffer.
U-Boot's strncpy does not do that: * Note that unlike userspace strncpy, this does not %NUL-pad the buffer. * However, the result is not %NUL-terminated if the source exceeds * @count bytes.
But you are right that I should use strlcpy here.
Thanks.
n = strlcpy(buf, &env[val], len); if (n >= len) ...
is probably closer to what you want. But only if you can trust &env[val] to actually have a nul byte before going into lala land.
Rasmus
On 12/10/2021 14.00, Marek Behún wrote:
On Tue, 12 Oct 2021 13:41:43 +0200 Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 12/10/2021 13.04, Marek Behún wrote:
I understand why you want to avoid an open-coded copying, but strncpy is almost certainly the wrong tool for the job. Apart from not revealing anything about the actual length of the source string, it also has the well-known defect of zeroing the tail of the buffer in the (usual) case where the source is shorter than the buffer.
U-Boot's strncpy does not do that:
- Note that unlike userspace strncpy, this does not %NUL-pad the
buffer.
- However, the result is not %NUL-terminated if the source exceeds
- @count bytes.
Yeah, I just saw that. That's extremely dangerous, and can cause silent wrong code generation. The compiler knows the semantics of various standard functions, including strncpy, and can optimize accordingly. For example, look at
https://godbolt.org/z/855s7hsEE
In g1, we see that gcc has recognized strncpy() and open-codes it by two immediate stores (6384738 == 0x616c62 == ascii "bla"), including the trailing zero padding. Then in g2, gcc further realizes that the conditional is always false, hence folds the whole thing to "return -1" (though it does not manage to elide the then-dead stores to buf).
So, I wouldn't bet that in cases where the compiler _does_ end up emitting a call to strncpy() that it doesn't somehow also rely on the implementation actually honouring the semantics required by the C standard.
For another example, see
The only way it is ok for gcc to compile copy_and_do to the exact same machine code as copy_and_do2 is because it knows memcpy returns its dst argument, hence (in the x86-64 case) it can prepare the first argument to do_stuff via a "mov rdi, rax", instead of spilling and reloading p.
Well, U-Boot seems to pessimize the build with -fno-builtin making much of the above moot. But:
On pa-risc, until very recently, "prctl(PR_SET_NAME, "x"); ....; prctl(PR_GET_NAME)" was a simple way to read 14 bytes of kernel stack from userspace because pa-risc's strncpy didn't do that zeroing. Of course U-Boot doesn't really have problems with that kind of info-leak, but given the amount of code U-Boot imports from other projects, not least linux, there's certainly a fair chance that some of that code relies on strncpy's semantics for correctness.
Rasmus
From: Marek Behún marek.behun@nic.cz
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 marek.behun@nic.cz --- cmd/nvedit.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 51b9e4ffa4..455783dccc 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(const char *env, const char *s1, int i2) +static const char *matching_name_get_value(const char *p, const char *name) { - while (*s1 != '\0' && *s1 == env[i2++]) - if (*s1++ == '=') - return i2; + while (*name != '\0' && *name == *p++) + if (*name++ == '=') + return p;
- if (*s1 == '\0' && env[i2-1] == '=') - return i2; + if (*name == '\0' && p[-1] == '=') + return p;
- return -1; + return NULL; }
/* @@ -723,8 +723,7 @@ 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 (name == NULL || *name == '\0') return -1; @@ -734,26 +733,26 @@ int env_get_f(const char *name, char *buf, unsigned len) 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; + unsigned copied;
- for (nxt = i; env[nxt] != '\0'; ++nxt) - if (nxt >= CONFIG_ENV_SIZE) + for (nxt = p; *nxt != '\0'; ++nxt) + if (p - env >= CONFIG_ENV_SIZE) return -1;
- val = env_match(env, name, i); - if (val < 0) + value = matching_name_get_value(p, name); + if (value == NULL) continue;
- /* found; copy out */ - n = strncpy(buf, &env[val], len) - buf; - if (len && n == len) { + copied = strncpy(buf, value, len) - buf; + if (len && copied == len) { buf[len - 1] = '\0'; printf("env_buf [%u bytes] too small for value of "%s"\n", len, name); }
- return n; + return copied; }
return -1;
From: Marek Behún marek.behun@nic.cz
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 this file.
Signed-off-by: Marek Behún marek.behun@nic.cz --- cmd/nvedit.c | 182 --------------------------------------------------- env/common.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 182 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 455783dccc..3bb6e764c0 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -30,7 +30,6 @@ #include <env.h> #include <env_internal.h> #include <log.h> -#include <net.h> #include <search.h> #include <errno.h> #include <malloc.h> @@ -38,7 +37,6 @@ #include <asm/global_data.h> #include <linux/bitops.h> #include <u-boot/crc.h> -#include <watchdog.h> #include <linux/stddef.h> #include <asm/byteorder.h> #include <asm/io.h> @@ -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,124 +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; -} - -static const char *matching_name_get_value(const char *p, const char *name) -{ - while (*name != '\0' && *name == *p++) - if (*name++ == '=') - return p; - - if (*name == '\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, *nxt; - - if (name == NULL || *name == '\0') - return -1; - - 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) { - const char *value; - unsigned copied; - - for (nxt = p; *nxt != '\0'; ++nxt) - if (p - env >= CONFIG_ENV_SIZE) - return -1; - - value = matching_name_get_value(p, name); - if (value == NULL) - continue; - - copied = strncpy(buf, value, len) - buf; - if (len && copied == len) { - buf[len - 1] = '\0'; - printf("env_buf [%u bytes] too small for value of "%s"\n", - len, name); - } - - return copied; - } - - 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..b1b2989bc9 100644 --- a/env/common.c +++ b/env/common.c @@ -21,6 +21,8 @@ #include <malloc.h> #include <u-boot/crc.h> #include <dm/ofnode.h> +#include <net.h> +#include <watchdog.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -33,6 +35,184 @@ struct hsearch_data env_htab = { .change_ok = env_flags_validate, };
+/** + * 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; +} + +static const char *matching_name_get_value(const char *p, const char *name) +{ + while (*name != '\0' && *name == *p++) + if (*name++ == '=') + return p; + + if (*name == '\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, *nxt; + + if (name == NULL || *name == '\0') + return -1; + + 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) { + const char *value; + unsigned copied; + + for (nxt = p; *nxt != '\0'; ++nxt) + if (p - env >= CONFIG_ENV_SIZE) + return -1; + + value = matching_name_get_value(p, name); + if (value == NULL) + continue; + + copied = strncpy(buf, value, len) - buf; + if (len && copied == len) { + buf[len - 1] = '\0'; + printf("env_buf [%u bytes] too small for value of "%s"\n", + len, name); + } + + return copied; + } + + 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)
participants (3)
-
Marek Behún -
Marek Behún -
Rasmus Villemoes