Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/SataControllerDxe: Remove useless null check

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Mon, May 8, 2023 at 11:28 PM Mike Maslenkin <mike.maslenkin@gmail.com> wrote:
>
> Hello Pedro,
> Technically speaking  ASSERT (Private != NULL) doesn't cover this branch.
> It should crash before as result of UninstallMultipleProtocolInterfaces() call.
> Obviously it make no sense in release target (under normal condition
> when assertion is turned off), while this code does.
> But I would suggest to remove ASSERT (Private != NULL) as well since
> it is useless also.
> It needs to be very lucky to get NULL as result of BASE_CR(), but
> actually SATA_CONTROLLER_PRIVATE_DATA_FROM_THIS() and CR()
> definition care about this. There will be assert if signature doesn't
> match to dereferenced memory area before Private != NULL check.
>
> In fact, this patch just reduces indentation level by removing useless checks.

Hmm yes, I agree. I don't see how that ASSERT can realistically fire.
It should also be removed.
I'm keeping this patch as-is since it's essentially a cheap backport
from OVMF SataControllerDxe; maintainers, pull it if you'd like, or I
can send a v2 later on that cleans up that ASSERT as well.

The OVMF patch doesn't really depend on this, as this is just refactoring.

(sidenote: you're dropping all CCs accidentally)

-- 
Pedro