Dear AKASHI Takahiro,

In message <20190717082525.891-6-takahiro.aka...@linaro.org> you wrote:
> Please note that the aim of this patch is to illustrate how we can
> extend the existing backing storage drivers for env interfaces to
> support U-Boot environment context.
>
> We will be able to support more devices as well as more contexts
> in a similar way. Existing drivers still work exactly in the same
> way as before while they are not extended yet.
>
> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> ---
>  env/fat.c | 103 ++++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 84 insertions(+), 19 deletions(-)
>
> diff --git a/env/fat.c b/env/fat.c
> index 7f74c64dfe7e..e4a672d2730a 100644
> --- a/env/fat.c
> +++ b/env/fat.c
> @@ -30,23 +30,44 @@
>  # endif
>  #endif
>  
> +static struct evn_fat_context {
> +     char *interface;
> +     char *dev_and_part;
> +     char *file;
> +} fat_context[ENVCTX_COUNT] = {
> +#if defined(CONFIG_ENV_FAT_INTERFACE) && \
> +     defined(CONFIG_ENV_FAT_DEVICE_AND_PART) && defined(CONFIG_ENV_FAT_FILE)
> +     [ENVCTX_UBOOT] = {
> +             CONFIG_ENV_FAT_INTERFACE,
> +             CONFIG_ENV_FAT_DEVICE_AND_PART,
> +             CONFIG_ENV_FAT_FILE,
> +     },
> +#endif
> +};

Does it make sense to define this in a FAT specific way? I guess we
could come up with a common structure that covers all supported
storage devices and use this here.

Also, the "#if defined" looks really dangerous to me, as missing
#defines will go unnoticed, and in the end you are accessing
uninitialized data... 

Did you review the patchset for memory leaks? 


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
If you can't explain it to a six year old, you  don't  understand  it
yourself.                                           - Albert Einstein
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to