Re: [PATCH v2 07/13] env: Inline env_get_char() into it's only user

2021-10-14 Thread Simon Glass
On Wed, 13 Oct 2021 at 09:46, Marek Behún  wrote:
>
> From: Marek Behún 
>
> 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 
> ---
>  cmd/nvedit.c  | 28 ++--
>  env/env.c |  8 
>  env/nowhere.c |  5 ++---
>  include/env.h | 10 --
>  4 files changed, 16 insertions(+), 35 deletions(-)
>

in subject: typo it's should be its

Reviewed-by: Simon Glass 


[PATCH v2 07/13] env: Inline env_get_char() into it's only user

2021-10-13 Thread Marek Behún
From: Marek Behún 

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 
---
 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 a22445132b..9f0caceadf 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 == env_get_char(i2++) && *s1 != '\0')
+   while (*s1 == env[i2++] && *s1 != '\0')
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
  *
-- 
2.32.0