diff options
Diffstat (limited to 'package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch')
| -rw-r--r-- | package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch | 173 |
1 files changed, 173 insertions, 0 deletions
diff --git a/package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch b/package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch new file mode 100644 index 0000000000..dbb71bd433 --- /dev/null +++ b/package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch @@ -0,0 +1,173 @@ +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 |