Marvin, I will review your patches this week.
-Jaben > -----Original Message----- > From: Marvin Häuser [mailto:[email protected]] > Sent: Thursday, May 19, 2016 1:20 PM > To: [email protected] > Cc: Carsey, Jaben <[email protected]> > Subject: RE: [edk2] [PATCH v1 1/1] ShellPkg/App: Fix memory leak and save > resources. > Importance: High > > 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
