Got it. Thanks. -Shumin
-----Original Message----- From: Ni, Ruiyu Sent: Wednesday, December 23, 2015 10:28 PM To: Qiu, Shumin; [email protected] Cc: Carsey, Jaben; Qiu, Shumin Subject: RE: [edk2] [PATCH v2] ShellPkg: Fix memory leak when running Shell script. Reviewed-by: Ruiyu Ni <[email protected]> Please separate the patch to two commits when you check in: The change to DoShellPrompt() is to fix the TAB-auto-completion memory leak. -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Qiu Shumin Sent: Wednesday, December 23, 2015 10:16 PM To: [email protected] Cc: Carsey, Jaben <[email protected]>; Ni, Ruiyu <[email protected]>; Qiu, Shumin <[email protected]> Subject: [edk2] [PATCH v2] ShellPkg: Fix memory leak when running Shell script. When we run following script in Shell: " for %a run (1 200) echo %a memmap endfor " We may find memory leak in system. This patch free buffer in 'BufferToFreeList' to avoid this issue. Cc: Jaben Carsey <[email protected]> Cc: Ruiyu Ni <[email protected]> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Qiu Shumin <[email protected]> --- ShellPkg/Application/Shell/Shell.c | 41 ++++++++++++++++++++++++++++++++++++++ ShellPkg/Application/Shell/Shell.h | 23 +++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c index 41c8a03..932a3a6 100644 --- a/ShellPkg/Application/Shell/Shell.c +++ b/ShellPkg/Application/Shell/Shell.c @@ -1205,6 +1205,7 @@ DoShellPrompt ( CONST CHAR16 *CurDir; UINTN BufferSize; EFI_STATUS Status; + LIST_ENTRY OldBufferList; CurDir = NULL; @@ -1218,6 +1219,7 @@ DoShellPrompt ( return EFI_OUT_OF_RESOURCES; } + SaveBufferList(&OldBufferList); CurDir = ShellInfoObject.NewEfiShellProtocol->GetEnv(L"cwd"); // @@ -1247,6 +1249,7 @@ DoShellPrompt ( // // Done with this command // + RestoreBufferList(&OldBufferList); FreePool (CmdLine); return Status; } @@ -1276,6 +1279,36 @@ AddBufferToFreeList( return (Buffer); } + +/** + Create a new buffer list and stores the old one to OldBufferList + + @param OldBufferList The temporary list head used to store the nodes in BufferToFreeList. +**/ +VOID +SaveBufferList ( + OUT LIST_ENTRY *OldBufferList + ) +{ + CopyMem (OldBufferList, &ShellInfoObject.BufferToFreeList.Link, +sizeof (LIST_ENTRY)); + InitializeListHead (&ShellInfoObject.BufferToFreeList.Link); +} + +/** + Restore previous nodes into BufferToFreeList . + + @param OldBufferList The temporary list head used to store the nodes in BufferToFreeList. +**/ +VOID +RestoreBufferList ( + IN OUT LIST_ENTRY *OldBufferList + ) +{ + FreeBufferList (&ShellInfoObject.BufferToFreeList); + CopyMem (&ShellInfoObject.BufferToFreeList.Link, OldBufferList, +sizeof (LIST_ENTRY)); } + + /** Add a buffer to the Line History List @@ -2656,6 +2689,7 @@ RunScriptFileHandle ( CONST CHAR16 *CurDir; UINTN LineCount; CHAR16 LeString[50]; + LIST_ENTRY OldBufferList; ASSERT(!ShellCommandGetScriptExit()); @@ -2758,6 +2792,8 @@ RunScriptFileHandle ( PrintBuffSize/sizeof(CHAR16) - 1 ); + SaveBufferList(&OldBufferList); + // // NULL out comments // @@ -2892,15 +2928,19 @@ RunScriptFileHandle ( ShellCommandRegisterExit(FALSE, 0); Status = EFI_SUCCESS; + RestoreBufferList(&OldBufferList); break; } if (ShellGetExecutionBreakFlag()) { + RestoreBufferList(&OldBufferList); break; } if (EFI_ERROR(Status)) { + RestoreBufferList(&OldBufferList); break; } if (ShellCommandGetExit()) { + RestoreBufferList(&OldBufferList); break; } } @@ -2919,6 +2959,7 @@ RunScriptFileHandle ( NewScriptFile->CurrentCommand->Reset = TRUE; } } + RestoreBufferList(&OldBufferList); } diff --git a/ShellPkg/Application/Shell/Shell.h b/ShellPkg/Application/Shell/Shell.h index 5726320..351b941 100644 --- a/ShellPkg/Application/Shell/Shell.h +++ b/ShellPkg/Application/Shell/Shell.h @@ -382,5 +382,28 @@ TrimSpaces( IN CHAR16 **String ); +/** + + Create a new buffer list and stores the old one to OldBufferList + + @param OldBufferList The temporary list head used to store the nodes in BufferToFreeList. +**/ +VOID +SaveBufferList ( + OUT LIST_ENTRY *OldBufferList + ); + +/** + Restore previous nodes into BufferToFreeList . + + @param OldBufferList The temporary list head used to store the nodes in BufferToFreeList. +**/ +VOID +RestoreBufferList ( + IN OUT LIST_ENTRY *OldBufferList + ); + + + #endif //_SHELL_INTERNAL_HEADER_ -- 1.9.5.msysgit.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
