[PATCH v2 3/9] env: correctly handle result in env_init

[Tom Rini <trini@konsulko.com>]

On Tue, Jun 16, 2020 at 09:40:42AM +0200, Patrick Delaunay wrote:

> Don't return error with ret=-ENOENT when the optional ops drv->init
> is absent but only if env_driver_lookup doesn't found driver.
> 
> This patch correct an issue for the code
>   if (!env_init())
>      env_load()
> When only ext4 is supported (CONFIG_ENV_IS_IN_EXT4),
> as the backend env/ext4.c doesn't define an ops .init
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
> 
> (no changes since v1)
> 
>  env/env.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/env/env.c b/env/env.c
> index dcc25c030b..819c88f729 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -295,7 +295,10 @@ int env_init(void)
>  	int prio;
>  
>  	for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); prio++) {
> -		if (!drv->init || !(ret = drv->init()))
> +		ret = 0;
> +		if (drv->init)
> +			ret = drv->init();
> +		if (!ret)
>  			env_set_inited(drv->location);
>  
>  		debug("%s: Environment %s init done (ret=%d)\n", __func__,

I'm adding in Marek here because this reminds me of similar questions /
concerns I had looking in to his series.  At root, I think we're not
being consistent in each of our env backing implementations about where
flags such as ENV_VALID are set, and return values / checks of
functions.

Just outside of the start of the patch context here, we set ret to
-ENOENT and just past this, if still -ENOENT we say ENV_VALID and point
at the default environment.

But, I don't follow the patch commit message here.  If we don't have
drv->init we call env_set_inited(drv->location) but we won't have change
ret to 0, which means that later on down the function we go back to
default environment.

So isn't this a problem in most environment cases then?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200618/e4e8eac0/attachment.sig>