+++ /dev/null
-From 07c79dd5fdeaefb39c9e7a97f3b66de63109a18d Mon Sep 17 00:00:00 2001
-From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
-Date: Wed, 12 Jan 2022 12:47:06 +0100
-Subject: [PATCH] fw_env: simplify logic & code paths in the fw_env_open()
-MIME-Version: 1.0
-Content-Type: text/plain; charset=UTF-8
-Content-Transfer-Encoding: 8bit
-
-Environment variables can be stored in two formats:
-1. Single entry with header containing CRC32
-2. Two entries with extra flags field in each entry header
-
-For that reason fw_env_open() has two main code paths and there are
-pointers for CRC32/flags/data.
-
-Previous implementation was a bit hard to follow:
-1. It was checking for used format twice (in reversed order each time)
-2. It was setting "environment" global struct fields to some temporary
- values that required extra comments explaining it
-
-This change simplifies that code:
-1. It introduces two clear code paths
-2. It sets "environment" global struct fields values only once it really
- knows them
-
-To be fair there are *two* crc32() calls now and an extra pointer
-variable but that should be cheap enough and worth it.
-
-Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
----
- tools/env/fw_env.c | 77 +++++++++++++++++++---------------------------
- 1 file changed, 31 insertions(+), 46 deletions(-)
-
---- a/tools/env/fw_env.c
-+++ b/tools/env/fw_env.c
-@@ -1421,9 +1421,6 @@ int fw_env_open(struct env_opts *opts)
-
- int ret;
-
-- struct env_image_single *single;
-- struct env_image_redundant *redundant;
--
- if (!opts)
- opts = &default_opts;
-
-@@ -1439,40 +1436,37 @@ int fw_env_open(struct env_opts *opts)
- goto open_cleanup;
- }
-
-- /* read environment from FLASH to local buffer */
-- environment.image = addr0;
--
-- if (have_redund_env) {
-- redundant = addr0;
-- environment.crc = &redundant->crc;
-- environment.flags = &redundant->flags;
-- environment.data = redundant->data;
-- } else {
-- single = addr0;
-- environment.crc = &single->crc;
-- environment.flags = NULL;
-- environment.data = single->data;
-- }
--
- dev_current = 0;
-- if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) {
-+ if (flash_io(O_RDONLY, addr0, CUR_ENVSIZE)) {
- ret = -EIO;
- goto open_cleanup;
- }
-
-- crc0 = crc32(0, (uint8_t *)environment.data, ENV_SIZE);
--
-- crc0_ok = (crc0 == *environment.crc);
- if (!have_redund_env) {
-+ struct env_image_single *single = addr0;
-+
-+ crc0 = crc32(0, (uint8_t *)single->data, ENV_SIZE);
-+ crc0_ok = (crc0 == single->crc);
- if (!crc0_ok) {
- fprintf(stderr,
- "Warning: Bad CRC, using default environment\n");
-- memcpy(environment.data, default_environment,
-+ memcpy(single->data, default_environment,
- sizeof(default_environment));
- environment.dirty = 1;
- }
-+
-+ environment.image = addr0;
-+ environment.crc = &single->crc;
-+ environment.flags = NULL;
-+ environment.data = single->data;
- } else {
-- flag0 = *environment.flags;
-+ struct env_image_redundant *redundant0 = addr0;
-+ struct env_image_redundant *redundant1;
-+
-+ crc0 = crc32(0, (uint8_t *)redundant0->data, ENV_SIZE);
-+ crc0_ok = (crc0 == redundant0->crc);
-+
-+ flag0 = redundant0->flags;
-
- dev_current = 1;
- addr1 = calloc(1, CUR_ENVSIZE);
-@@ -1483,14 +1477,9 @@ int fw_env_open(struct env_opts *opts)
- ret = -ENOMEM;
- goto open_cleanup;
- }
-- redundant = addr1;
-+ redundant1 = addr1;
-
-- /*
-- * have to set environment.image for flash_read(), careful -
-- * other pointers in environment still point inside addr0
-- */
-- environment.image = addr1;
-- if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) {
-+ if (flash_io(O_RDONLY, addr1, CUR_ENVSIZE)) {
- ret = -EIO;
- goto open_cleanup;
- }
-@@ -1518,18 +1507,12 @@ int fw_env_open(struct env_opts *opts)
- goto open_cleanup;
- }
-
-- crc1 = crc32(0, (uint8_t *)redundant->data, ENV_SIZE);
-+ crc1 = crc32(0, (uint8_t *)redundant1->data, ENV_SIZE);
-
-- crc1_ok = (crc1 == redundant->crc);
-- flag1 = redundant->flags;
-+ crc1_ok = (crc1 == redundant1->crc);
-+ flag1 = redundant1->flags;
-
-- /*
-- * environment.data still points to ((struct
-- * env_image_redundant *)addr0)->data. If the two
-- * environments differ, or one has bad crc, force a
-- * write-out by marking the environment dirty.
-- */
-- if (memcmp(environment.data, redundant->data, ENV_SIZE) ||
-+ if (memcmp(redundant0->data, redundant1->data, ENV_SIZE) ||
- !crc0_ok || !crc1_ok)
- environment.dirty = 1;
-
-@@ -1540,7 +1523,7 @@ int fw_env_open(struct env_opts *opts)
- } else if (!crc0_ok && !crc1_ok) {
- fprintf(stderr,
- "Warning: Bad CRC, using default environment\n");
-- memcpy(environment.data, default_environment,
-+ memcpy(redundant0->data, default_environment,
- sizeof(default_environment));
- environment.dirty = 1;
- dev_current = 0;
-@@ -1586,13 +1569,15 @@ int fw_env_open(struct env_opts *opts)
- */
- if (dev_current) {
- environment.image = addr1;
-- environment.crc = &redundant->crc;
-- environment.flags = &redundant->flags;
-- environment.data = redundant->data;
-+ environment.crc = &redundant1->crc;
-+ environment.flags = &redundant1->flags;
-+ environment.data = redundant1->data;
- free(addr0);
- } else {
- environment.image = addr0;
-- /* Other pointers are already set */
-+ environment.crc = &redundant0->crc;
-+ environment.flags = &redundant0->flags;
-+ environment.data = redundant0->data;
- free(addr1);
- }
- #ifdef DEBUG
projects / openwrt / staging / stintel.git / blobdiff