Hao: The change is good to me. Reviewed-by: Liming Gao <[email protected]>
>-----Original Message----- >From: Wu, Hao A >Sent: Friday, April 26, 2019 1:39 PM >To: Ard Biesheuvel <[email protected]>; Kinney, Michael D ><[email protected]>; Gao, Liming <[email protected]>; Wang, >Jian J <[email protected]> >Cc: edk2-devel-groups-io <[email protected]> >Subject: RE: [PATCH v2] MdeModulePkg/DxeCore: Please static checker for >false report > >> -----Original Message----- >> From: Ard Biesheuvel [mailto:[email protected]] >> Sent: Wednesday, April 24, 2019 3:07 PM >> To: Wu, Hao A >> Cc: edk2-devel-groups-io; Kinney, Michael D; Gao, Liming; Wang, Jian J >> Subject: Re: [PATCH v2] MdeModulePkg/DxeCore: Please static checker for >> false report >> >> On Wed, 24 Apr 2019 at 07:05, Hao Wu <[email protected]> wrote: >> > >> > After commit 57df17fe26, some static check reports suspicous NULL >pointer >> > deference at line: >> > >> > Entry->MachineType = Entry->Emulator->MachineType; >> > ^^^^^^^^^^^^^^^ >> > >> > within function PeCoffEmuProtocolNotify(). >> > >> > However, 'Entry->Emulator' is guaranteed to have a non-NULL value when >> > previous call to the CoreHandleProtocol() returns EFI_SUCCESS. >> > >> > This commit will re-write the return status check for CoreHandleProtocol() >> > to add explicit NULL pointer check for protocol instance pointer. >> > >> > Cc: Ard Biesheuvel <[email protected]> >> > Cc: Michael D Kinney <[email protected]> >> > Cc: Liming Gao <[email protected]> >> > Cc: Jian J Wang <[email protected]> >> > Signed-off-by: Hao Wu <[email protected]> >> >> Again, I think it is rather unfortunate that we need code changes such >> as this one just to remove warnings from a flawed static analyzer. But >> the change looks correct to me, so >> >> Acked-by: Ard Biesheuvel <[email protected]> > >Thanks Ard, > >Hello Mike, Liming and Jian, > >Do you have additional feedbacks on this patch? >If not, I plan to push it with the 'Ack' tag from Ard. > >Best Regards, >Hao Wu > >> >> > --- >> > MdeModulePkg/Core/Dxe/Image/Image.c | 23 ++++++++++++-------- >> > 1 file changed, 14 insertions(+), 9 deletions(-) >> > >> > diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c >> b/MdeModulePkg/Core/Dxe/Image/Image.c >> > index 08306a73fd..de5b8bed27 100644 >> > --- a/MdeModulePkg/Core/Dxe/Image/Image.c >> > +++ b/MdeModulePkg/Core/Dxe/Image/Image.c >> > @@ -134,12 +134,14 @@ PeCoffEmuProtocolNotify ( >> > IN VOID *Context >> > ) >> > { >> > - EFI_STATUS Status; >> > - UINTN BufferSize; >> > - EFI_HANDLE EmuHandle; >> > - EMULATOR_ENTRY *Entry; >> > + EFI_STATUS Status; >> > + UINTN BufferSize; >> > + EFI_HANDLE EmuHandle; >> > + EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *Emulator; >> > + EMULATOR_ENTRY *Entry; >> > >> > EmuHandle = NULL; >> > + Emulator = NULL; >> > >> > while (TRUE) { >> > BufferSize = sizeof (EmuHandle); >> > @@ -157,16 +159,19 @@ PeCoffEmuProtocolNotify ( >> > return; >> > } >> > >> > - Entry = AllocateZeroPool (sizeof (*Entry)); >> > - ASSERT (Entry != NULL); >> > - >> > Status = CoreHandleProtocol ( >> > EmuHandle, >> > &gEdkiiPeCoffImageEmulatorProtocolGuid, >> > - (VOID **)&Entry->Emulator >> > + (VOID **)&Emulator >> > ); >> > - ASSERT_EFI_ERROR (Status); >> > + if (EFI_ERROR (Status) || Emulator == NULL) { >> > + continue; >> > + } >> > + >> > + Entry = AllocateZeroPool (sizeof (*Entry)); >> > + ASSERT (Entry != NULL); >> > >> > + Entry->Emulator = Emulator; >> > Entry->MachineType = Entry->Emulator->MachineType; >> > >> > InsertTailList (&mAvailableEmulators, &Entry->Link); >> > -- >> > 2.12.0.windows.1 >> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39632): https://edk2.groups.io/g/devel/message/39632 Mute This Topic: https://groups.io/mt/31318926/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
