On Mon, May 8, 2023 at 11:28 PM Mike Maslenkin <mike.maslen...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#104299): https://edk2.groups.io/g/devel/message/104299 Mute This Topic: https://groups.io/mt/98771780/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
