[PATCH] efi_loader: fix an IS_ERR() vs NULL check

[Dan Carpenter]

On Thu, Jul 27, 2023 at 10:25:55AM +0200, Heinrich Schuchardt wrote:
> On 7/27/23 09:16, Dan Carpenter wrote:
> > The efi_parse_pkcs7_header() function returns NULL on error so the check
> > for IS_ERR() should be changed to a NULL check.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter at linaro.org>
> > ---
> >   lib/efi_loader/efi_capsule.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 7a6f195cbc02..c98cff812f10 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -368,9 +368,8 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s
> >   					     auth_hdr->auth_info.hdr.dwLength
> >   					     - sizeof(auth_hdr->auth_info),
> >   					     &buf);
> > -	if (IS_ERR(capsule_sig)) {
> > +	if (!capsule_sig) {
> 
> Thanks for addressing this bug.
> 
> Can we get rid of all the IS_ERR() in efi_capsule_authenticate() and use
> IS_ERR_OR_NULL() here?

Ugh...  Please don't do that.  In u-boot it probably doesn't matter but
in the kernel they mean very different things.  A NULL in that context
means the feature is optional and disabled deliberately but an error
pointer means there was an error.

int blink_LEDS()
{
	led = get_leds();
	if (IS_ERR_OR_NULL(led)) {
		if (IS_ERR(led))
			print("LEDs are dead\n");
		return PTR_ERR(led); <-- returns success if led is NULL
	}

	return led->blink();
}

So checking efi_capsule_authenticate() for IS_ERR_OR_NULL() means it's
optional and more options means more complications.

regards,
dan carpenter