Hey Jaben, As I mentioned in 3), there is still the CreateFileInterfaceMem() resource leak if the list is not empty, so this is definitely still a todo, I do not know how to solve (yet?). I only kept the ASSERT() because it was present before, I don't really see the point in stopping execution there to be honest.
By the way, if you have time, please review my other two patches as well: [PATCH v1] ShellPkg: Also accept gEfiUnicodeCollation2ProtocolGuid for parsing. [PATCH v1] ShellPkg/Bcfg: Add support for 'addp' command. Thank you for your time! Regards, Marvin. > -----Original Message----- > From: Carsey, Jaben [mailto:[email protected]] > Sent: Thursday, May 19, 2016 10:09 PM > To: Marvin Häuser <[email protected]>; edk2- > [email protected] > Cc: Qiu, Shumin <[email protected]>; Carsey, Jaben > <[email protected]> > Subject: RE: [edk2] [PATCH v1 1/1] ShellPkg/App: Fix memory leak and save > resources. > > Can we safely remove the ASSERt now? > > > -----Original Message----- > > From: edk2-devel [mailto:[email protected]] On Behalf Of > > Marvin Häuser > > Sent: Thursday, May 19, 2016 12:04 PM > > To: [email protected] > > Cc: Carsey, Jaben <[email protected]>; Qiu, Shumin > > <[email protected]> > > Subject: [edk2] [PATCH v1 1/1] ShellPkg/App: Fix memory leak and save > > resources. > > Importance: High > > > > 1) RunSplitCommand() allocates the initial SplitStdOut via > > CreateFileInterfaceMem(). Free SplitStdIn after the swap to fix > > the memory leak. > > > > 2) In RunSplitCommand(), SplitStdOut is checked for equality with > > StdIn. This cannot happen due to the if-check within the swap. > > Hence remove it. > > > > 3) UefiMain() doesn't free SplitList. Delete all list entries and > > reinitialize the list when in DEBUG. This does not include the > > CreateFileInterfaceMem()-allocated SplitStd mentioned in 1), so > > keep the ASSERT() until resolved. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Marvin Haeuser <[email protected]> > > --- > > ShellPkg/Application/Shell/Shell.c | 16 ++++++++++++++-- > > 1 file changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/ShellPkg/Application/Shell/Shell.c > > b/ShellPkg/Application/Shell/Shell.c > > index 47b3118ea701..a562c9a2c0bc 100644 > > --- a/ShellPkg/Application/Shell/Shell.c > > +++ b/ShellPkg/Application/Shell/Shell.c > > @@ -342,6 +342,7 @@ UefiMain ( > > UINTN Size; > > EFI_HANDLE ConInHandle; > > EFI_SIMPLE_TEXT_INPUT_PROTOCOL *OldConIn; > > + SPLIT_LIST *Split; > > > > if (PcdGet8(PcdShellSupportLevel) > 3) { > > return (EFI_UNSUPPORTED); > > @@ -675,7 +676,17 @@ FreeResources: > > } > > > > if (!IsListEmpty(&ShellInfoObject.SplitList.Link)){ > > - ASSERT(FALSE); ///@todo finish this de-allocation. > > + ASSERT(FALSE); ///@todo finish this de-allocation (free > > + SplitStdIn/Out > > when needed). > > + > > + for ( Split = (SPLIT_LIST*)GetFirstNode > > (&ShellInfoObject.SplitList.Link) > > + ; !IsNull (&ShellInfoObject.SplitList.Link, &Split->Link) > > + ; Split = (SPLIT_LIST *)GetNextNode > > + (&ShellInfoObject.SplitList.Link, > > &Split->Link) > > + ) { > > + RemoveEntryList (&Split->Link); > > + FreePool (Split); > > + } > > + > > + DEBUG_CODE (InitializeListHead > > + (&ShellInfoObject.SplitList.Link);); > > } > > > > if (ShellInfoObject.ShellInitSettings.FileName != NULL) { @@ > > -1736,11 +1747,12 @@ RunSplitCommand( > > // > > // Note that the original StdIn is now the StdOut... > > // > > - if (Split->SplitStdOut != NULL && Split->SplitStdOut != StdIn) { > > + if (Split->SplitStdOut != NULL) { > > ShellInfoObject.NewEfiShellProtocol- > > >CloseFile(ConvertShellHandleToEfiFileProtocol(Split->SplitStdOut)); > > } > > if (Split->SplitStdIn != NULL) { > > ShellInfoObject.NewEfiShellProtocol- > > >CloseFile(ConvertShellHandleToEfiFileProtocol(Split->SplitStdIn)); > > + FreePool (Split->SplitStdIn); > > } > > > > FreePool(Split); > > -- > > 2.7.4.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > [email protected] > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
