For double buffering to work, the target buffer must always be big enough to hold all data. This can only be ensured if buffers are of equal size, otherwise one must be smaller and we risk data loss when copying from the bigger to the smaller buffer.

Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- tools/env/fw_env.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 692abda..b1c8217 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1382,18 +1382,19 @@ static int parse_config(struct env_opts *opts) return -1; }

- if (HaveRedundEnv && stat (DEVNAME (1), &st)) { - fprintf (stderr, - "Cannot access MTD device %s: %s\n", - DEVNAME (1), strerror (errno)); - return -1; - } + if (HaveRedundEnv) { + if (stat(DEVNAME(1), &st)) { + fprintf(stderr, + "Cannot access MTD device %s: %s\n", + DEVNAME(1), strerror(errno)); + return -1; + }

- if (HaveRedundEnv && ENVSIZE(0) != ENVSIZE(1)) { - ENVSIZE(0) = ENVSIZE(1) = min(ENVSIZE(0), ENVSIZE(1)); - fprintf(stderr, - "Redundant environments have inequal size, set to 0x%08lx\n", - ENVSIZE(1)); + if (ENVSIZE(0) != ENVSIZE(1)) { + fprintf(stderr, + "Redundant environments have unequal size"); + return -1; + } }

usable_envsize = CUR_ENVSIZE - sizeof(uint32_t);

there are two groups of functions: - application ready tools: fw_setenv/fw_getenv/fw_parse_script these are used, when creating a single binary containing multiple tools (busybox like) - file access like: open/read/write/close above functions are implemented on top of these. applications can use those to modify several variables without creating a temporary batch script file tested with "./scripts/kernel-doc -html -v tools/env/fw_env.h"

Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- tools/env/fw_env.c | 2 +- tools/env/fw_env.h | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 115 insertions(+), 2 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index b1c8217..fe479e9 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -483,7 +483,7 @@ int fw_setenv(int argc, char *argv[], struct env_opts *opts) valc = argc - 1;

if (env_flags_validate_env_set_params(name, valv, valc) < 0) - return 1; + return -1;

len = 0; for (i = 0; i < valc; ++i) { diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h index dac964d..436eca9 100644 --- a/tools/env/fw_env.h +++ b/tools/env/fw_env.h @@ -67,12 +67,125 @@ struct env_opts {

int parse_aes_key(char *key, uint8_t *bin_key);

+/** + * fw_printenv() - print one or several environment variables + * + * @argc: number of variables names to be printed, prints all if 0 + * @argv: array of variable names to be printed, if argc != 0 + * @value_only: do not repeat the variable name, print the bare value, + * only one variable allowed with this option, argc must be 1 + * @opts: encryption key, configuration file, defaults are used if NULL + * + * Description: + * Uses fw_env_open, fw_getenv + * + * Return: + * 0 on success, -1 on failure (modifies errno) + */ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts); -char *fw_getenv(char *name); + +/** + * fw_setenv() - adds or removes one variable to the environment + * + * @argc: number of strings in argv, argv[0] is variable name, + * argc==1 means erase variable, argc > 1 means add a variable + * @argv: argv[0] is variable name, argv[1..argc-1] are concatenated separated + * by single blank and set as the new value of the variable + * @opts: how to retrieve environment from flash, defaults are used if NULL + * + * Description: + * Uses fw_env_open, fw_env_write, fw_env_close + * + * Return: + * 0 on success, -1 on failure (modifies errno) + * + * ERRORS: + * EROFS - some variables ("ethaddr", "serial#") cannot be modified + */ int fw_setenv(int argc, char *argv[], struct env_opts *opts); + +/** + * fw_parse_script() - adds or removes multiple variables with a batch script + * + * @fname: batch script file name + * @opts: encryption key, configuration file, defaults are used if NULL + * + * Description: + * Uses fw_env_open, fw_env_write, fw_env_close + * + * Return: + * 0 success, -1 on failure (modifies errno) + * + * Script Syntax: + * + * key [ [space]+ value] + * + * lines starting with '#' treated as comment + * + * A variable without value will be deleted. Any number of spaces are allowed + * between key and value. The value starts with the first non-space character + * and ends with newline. No comments allowed on these lines. Spaces inside + * the value are preserved verbatim. + * + * Script Example: + * + * netdev eth0 + * + * kernel_addr 400000 + * + * foo spaces are copied verbatim + * + * # delete variable bar + * + * bar + */ int fw_parse_script(char *fname, struct env_opts *opts); + + +/** + * fw_env_open() - read enviroment from flash into RAM cache + * + * @opts: encryption key, configuration file, defaults are used if NULL + * + * Return: + * 0 on success, -1 on failure (modifies errno) + */ int fw_env_open(struct env_opts *opts); + +/** + * fw_getenv() - lookup variable in the RAM cache + * + * @name: variable to be searched + * Return: + * pointer to start of value, NULL if not found + */ +char *fw_getenv(char *name); + +/** + * fw_env_write() - modify a variable held in the RAM cache + * + * @name: variable + * @value: delete variable if NULL, otherwise create or overwrite the variable + * + * This is called in sequence to update the environment in RAM without updating + * the copy in flash after each set + * + * Return: + * 0 on success, -1 on failure (modifies errno) + * + * ERRORS: + * EROFS - some variables ("ethaddr", "serial#") cannot be modified + */ int fw_env_write(char *name, char *value); + +/** + * fw_env_close - write the environment from RAM cache back to flash + * + * @opts: encryption key, configuration file, defaults are used if NULL + * + * Return: + * 0 on success, -1 on failure (modifies errno) + */ int fw_env_close(struct env_opts *opts);

unsigned long crc32(unsigned long, const unsigned char *, unsigned);

forward declaration not needed when re-ordered

Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- tools/env/fw_env.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index fe479e9..f155c12 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -121,7 +121,6 @@ static unsigned char obsolete_flag = 0; #include <env_default.h>

static int flash_io (int mode); -static char *envmatch (char * s1, char * s2); static int parse_config(struct env_opts *opts);

#if defined(CONFIG_FILE) @@ -147,6 +146,24 @@ static char *skip_blanks(char *s) }

/* + * s1 is either a simple 'name', or a 'name=value' pair. + * s2 is a 'name=value' pair. + * If the names match, return the value of s2, else NULL. + */ +static char *envmatch(char *s1, char *s2) +{ + if (s1 == NULL || s2 == NULL) + return NULL; + + while (*s1 == *s2++) + if (*s1++ == '=') + return s2; + if (*s1 == '\0' && *(s2 - 1) == '=') + return s2; + return NULL; +} + +/** * Search the environment for a variable. * Return the value, if found, or NULL, if not found. */ @@ -1121,25 +1138,6 @@ exit: }

/* - * s1 is either a simple 'name', or a 'name=value' pair. - * s2 is a 'name=value' pair. - * If the names match, return the value of s2, else NULL. - */ - -static char *envmatch (char * s1, char * s2) -{ - if (s1 == NULL || s2 == NULL) - return NULL; - - while (*s1 == *s2++) - if (*s1++ == '=') - return s2; - if (*s1 == '\0' && *(s2 - 1) == '=') - return s2; - return NULL; -} - -/* * Prevent confusion if running from erased flash memory */ int fw_env_open(struct env_opts *opts)

Try to avoid adhoc iteration of the environment. Reuse fw_getenv to find the variables that should be printed. Only use open-coded iteration when printing all variables. For backwards compatibility, keep emitting a newline when printing with value_only.

Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- tools/env/fw_env.c | 44 +++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 27 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index f155c12..8b3a132 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -249,9 +249,14 @@ int parse_aes_key(char *key, uint8_t *bin_key) */ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts) { - char *env, *nxt; int i, rc = 0;

+ if (value_only && argc != 1) { + fprintf(stderr, + "## Error: `-n' option requires exactly one argument\n"); + return -1; + } + if (!opts) opts = &default_opts;

@@ -259,6 +264,7 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts) return -1;

if (argc == 0) { /* Print all env variables */ + char *env, *nxt; for (env = environment.data; *env; env = nxt + 1) { for (nxt = env; *nxt; ++nxt) { if (nxt >= &environment.data[ENV_SIZE]) { @@ -273,39 +279,23 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts) return 0; }

- if (value_only && argc != 1) { - fprintf(stderr, - "## Error: `-n' option requires exactly one argument\n"); - return -1; - } - - for (i = 0; i < argc; ++i) { /* print single env variables */ + for (i = 0; i < argc; ++i) { /* print a subset of env variables */ char *name = argv[i]; char *val = NULL;

- for (env = environment.data; *env; env = nxt + 1) { - - for (nxt = env; *nxt; ++nxt) { - if (nxt >= &environment.data[ENV_SIZE]) { - fprintf (stderr, "## Error: " - "environment not terminated\n"); - return -1; - } - } - val = envmatch (name, env); - if (val) { - if (!value_only) { - fputs (name, stdout); - putc ('=', stdout); - } - puts (val); - break; - } - } + val = fw_getenv(name); if (!val) { fprintf (stderr, "## Error: "%s" not defined\n", name); rc = -1; + continue; } + + if (value_only) { + puts(val); + break; + } + + printf("%s=%s\n", name, val); }

return rc;