[1/2] env: common: Return specific error code on bad CRC

Message ID 20190118191904.634-2-semen.protsenko@linaro.org
State Accepted
Commit 49d04c589c669401bc45a491a64aa68762b20efe
Headers show
Series
  • env: Fix "env save" to malformed environment
Related show

Commit Message

Sam Protsenko Jan. 18, 2019, 7:19 p.m.
Callers of env_import*() functions might want to check the case when we
have incorrect environment (with bad CRC). For example, when environment
location is being defined in env_load(), call chain may look like this:

    env_load() -> drv->load() = env_mmc_load() -> env_import()

Return code will be passed from env_import() all way up to env_load().
Right now both env_mmc_load() and env_import() return -EIO error code,
so env_load() can't differentiate between two cases:
  1. Driver reports the error, because device is not accessible
  2. Device is actually accessible, but environment is broken

Let's return -ENOMSG in env_import(), so we can distinguish two cases
mentioned above. It will make it possible to continue working with "bad
CRC" environment (like doing "env save"), instead of considering it not
functional (implemented in subsequent patch).

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
 env/common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Simon Goldschmidt Jan. 21, 2019, 1:36 p.m. | #1
Am 18.01.2019 um 20:19 schrieb Sam Protsenko:
> Callers of env_import*() functions might want to check the case when we
> have incorrect environment (with bad CRC). For example, when environment
> location is being defined in env_load(), call chain may look like this:
> 
>      env_load() -> drv->load() = env_mmc_load() -> env_import()
> 
> Return code will be passed from env_import() all way up to env_load().
> Right now both env_mmc_load() and env_import() return -EIO error code,
> so env_load() can't differentiate between two cases:
>    1. Driver reports the error, because device is not accessible
>    2. Device is actually accessible, but environment is broken
> 
> Let's return -ENOMSG in env_import(), so we can distinguish two cases
> mentioned above. It will make it possible to continue working with "bad
> CRC" environment (like doing "env save"), instead of considering it not
> functional (implemented in subsequent patch).
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>

Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

> ---
>   env/common.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/env/common.c b/env/common.c
> index d1a6a52860..324502ed82 100644
> --- a/env/common.c
> +++ b/env/common.c
> @@ -115,7 +115,7 @@ int env_import(const char *buf, int check)
>   
>   		if (crc32(0, ep->data, ENV_SIZE) != crc) {
>   			set_default_env("bad CRC", 0);
> -			return -EIO;
> +			return -ENOMSG; /* needed for env_load() */
>   		}
>   	}
>   
> @@ -169,7 +169,7 @@ int env_import_redund(const char *buf1, int buf1_read_fail,
>   
>   	if (!crc1_ok && !crc2_ok) {
>   		set_default_env("bad CRC", 0);
> -		return -EIO;
> +		return -ENOMSG; /* needed for env_load() */
>   	} else if (crc1_ok && !crc2_ok) {
>   		gd->env_valid = ENV_VALID;
>   	} else if (!crc1_ok && crc2_ok) {
>

Patch

diff --git a/env/common.c b/env/common.c
index d1a6a52860..324502ed82 100644
--- a/env/common.c
+++ b/env/common.c
@@ -115,7 +115,7 @@  int env_import(const char *buf, int check)
 
 		if (crc32(0, ep->data, ENV_SIZE) != crc) {
 			set_default_env("bad CRC", 0);
-			return -EIO;
+			return -ENOMSG; /* needed for env_load() */
 		}
 	}
 
@@ -169,7 +169,7 @@  int env_import_redund(const char *buf1, int buf1_read_fail,
 
 	if (!crc1_ok && !crc2_ok) {
 		set_default_env("bad CRC", 0);
-		return -EIO;
+		return -ENOMSG; /* needed for env_load() */
 	} else if (crc1_ok && !crc2_ok) {
 		gd->env_valid = ENV_VALID;
 	} else if (!crc1_ok && crc2_ok) {