[PATCH] tools: imagetool: Remove unnecessary check from toc0_verify_cert_item()

Andre Przywara andre.przywara at arm.com
Thu Aug 1 11:27:38 CEST 2024


On Thu,  1 Aug 2024 10:01:00 +0900
Seung-Woo Kim <sw0312.kim at samsung.com> wrote:

Hi,

> Remove unnecessary null check from toc0_verify_cert_item()
> because the array digest is always not null.

Do you mean it's always not NULL *as currently used*? Because digest is a
function parameter, so within the scope of this function definition can be
NULL.
I agree that *currently* there is only one caller, and this passes the
addresses of a local variable from the stack, though it's never NULL.

But I don't think this check hurts (looks like the compiler removes it
anyway), and it makes the code more robust. So is there any problem with
this check? Why do you want it to be removed?

Cheers,
Andre

> 
> Signed-off-by: Seung-Woo Kim <sw0312.kim at samsung.com>
> ---
>  tools/sunxi_toc0.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/sunxi_toc0.c b/tools/sunxi_toc0.c
> index 292649fe90f1..76693647a095 100644
> --- a/tools/sunxi_toc0.c
> +++ b/tools/sunxi_toc0.c
> @@ -444,7 +444,7 @@ static int toc0_verify_cert_item(const uint8_t *buf, uint32_t len, RSA *fw_key,
>  
>  	/* If a digest was provided, compare it to the embedded digest. */
>  	extension = &totalSequence->mainSequence.explicit3.extension;
> -	if (digest && memcmp(&extension->digest, digest, SHA256_DIGEST_LENGTH)) {
> +	if (memcmp(&extension->digest, digest, SHA256_DIGEST_LENGTH)) {
>  		pr_err("Wrong firmware digest in certificate\n");
>  		goto err;
>  	}



More information about the U-Boot mailing list