Hi, > -----Original Message----- > From: Andre Przywara <andre.przyw...@arm.com> > Sent: Thursday, August 1, 2024 6:28 PM > Subject: Re: [PATCH] tools: imagetool: Remove unnecessary check from > toc0_verify_cert_item() > > On Thu, 1 Aug 2024 10:01:00 +0900 > Seung-Woo Kim <sw0312....@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?
Because in my gcc-14 environment, it gives -Wnonnull-compare warning. tools/sunxi_toc0.c: In function 'toc0_verify_cert_item': tools/sunxi_toc0.c:447:12: warning: 'nonnull' argument 'digest' compared to NULL [-Wnonnull-compare] 447 | if (digest && memcmp(&extension->digest, digest, SHA256_DIGEST_LENGTH)) { | ^ Regards, - Seung-Woo Kim > > Cheers, > Andre > > > > > Signed-off-by: Seung-Woo Kim <sw0312....@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; > > }