diff mbox series

[11/16] board: stm32mp1: check env_get result in board_late_init

Message ID 20200331180330.11.Ic051e25812481db408f2431c7962da1db1f198fb@changeid
State Superseded
Delegated to: Patrick Delaunay
Headers show
Series [01/16] arm: stm32mp: update dependency for STM32_ETZPC | expand

Commit Message

Patrick Delaunay March 31, 2020, 4:04 p.m. UTC
This patch avoids crash in strcmp when the boot_device is not
present in environment (this case should be never occur)

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 board/st/stm32mp1/stm32mp1.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Patrice CHOTARD April 1, 2020, 8:06 a.m. UTC | #1
Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> This patch avoids crash in strcmp when the boot_device is not
> present in environment (this case should be never occur)
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 89a088cd28..fff4cef2c2 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -753,7 +753,8 @@ int board_late_init(void)
>  
>  	/* Check the boot-source to disable bootdelay */
>  	boot_device = env_get("boot_device");
> -	if (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb"))
> +	if (boot_device &&
> +	    (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb")))
>  		env_set("bootdelay", "0");
>  
>  	return 0;

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Thanks
Wolfgang Denk April 1, 2020, 11:39 a.m. UTC | #2
Dear Patrick Delaunay,

In message <20200331180330.11.Ic051e25812481db408f2431c7962da1db1f198fb@changeid> you wrote:
> This patch avoids crash in strcmp when the boot_device is not
> present in environment (this case should be never occur)
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 89a088cd28..fff4cef2c2 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -753,7 +753,8 @@ int board_late_init(void)
>  
>  	/* Check the boot-source to disable bootdelay */
>  	boot_device = env_get("boot_device");
> -	if (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb"))
> +	if (boot_device &&
> +	    (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb")))
>  		env_set("bootdelay", "0");

I think this is generally a bad idea.  You should have more respect
to the intentions of your users.  If a user defines a specific
bootdelay setting in his environment, this must be respected.

I really hate vendors who believe they know better than me what I
want or what is good to me.  Please get rid of such stuff.

"bootdelay" is an environment variable that is intentionally user
definable. You must never mess with those.

Best regards,

Wolfgang Denk
diff mbox series

Patch

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 89a088cd28..fff4cef2c2 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -753,7 +753,8 @@  int board_late_init(void)
 
 	/* Check the boot-source to disable bootdelay */
 	boot_device = env_get("boot_device");
-	if (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb"))
+	if (boot_device &&
+	    (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb")))
 		env_set("bootdelay", "0");
 
 	return 0;