Re: [PATCH] MdeModulePkg/TerminalDxe: Avoid always append device path to *Dev

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Good idea😊

Mike,
What's your opinion?

Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, April 19, 2017 10:54 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Avoid always
> append device path to *Dev
> 
> Reviewed-by: Star Zeng <star.zeng@intel.com>
> 
> BTW: May the MatchDevicePaths could be proposed to DevicePathLib? :)
> 
> ---- MatchDevicePaths Matches (5 in 1 files) ---- ConsoleOption.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\BootMaintenanceManagerUi
> Lib):MatchDevicePaths (
> ConsoleOption.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\BootMaintenanceManagerUi
> Lib):      if (MatchDevicePaths (OutDevicePath, NewDevicePath)) {
> ConsoleOption.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\BootMaintenanceManagerUi
> Lib):      if (MatchDevicePaths (InpDevicePath, NewDevicePath)) {
> ConsoleOption.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\BootMaintenanceManagerUi
> Lib):      if (MatchDevicePaths (ErrDevicePath, NewDevicePath)) {
> ConsoleOption.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\BootMaintenanceManagerUi
> Lib):    NewConsoleContext->IsActive = MatchDevicePaths (
> ---- BmMatchDevicePaths Matches (4 in 4 files) ---- BmBoot.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\UefiBootManagerLib):  if
> (BmMatchDevicePaths (*CachedDevicePath, DevicePath)) {
> BmConsole.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\UefiBootManagerLib):    if
> (!BmMatchDevicePaths (NewDevicePath, CustomizedConDevicePath)) {
> BmMisc.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\UefiBootManagerLib):BmMa
> tchDevicePaths ( InternalBm.h
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Library\UefiBootManagerLib):BmMa
> tchDevicePaths (
> ---- BdsLibMatchDevicePaths Matches (11 in 8 files) ----
> BdsBoot.c
> (F:\GIT\edk2Git\edk2\IntelFrameworkModulePkg\Library\GenericBdsLib):
> if (BdsLibMatchDevicePaths (CachedDevicePath, BlockIoDevicePath)) {
> BdsBoot.c
> (F:\GIT\edk2Git\edk2\Vlv2TbltDevicePkg\Override\IntelFrameworkModule
> Pkg\Library\GenericBdsLib):        if (BdsLibMatchDevicePaths
> (CachedDevicePath, BlockIoDevicePath)) {
> BdsConsole.c
> (F:\GIT\edk2Git\edk2\IntelFrameworkModulePkg\Library\GenericBdsLib):
> if (!BdsLibMatchDevicePaths (NewDevicePath, CustomizedConDevicePath)) {
> BdsConsole.c
> (F:\GIT\edk2Git\edk2\Vlv2TbltDevicePkg\Override\IntelFrameworkModule
> Pkg\Library\GenericBdsLib):    if (!BdsLibMatchDevicePaths (NewDevicePath,
> CustomizedConDevicePath)) {
> BdsMisc.c
> (F:\GIT\edk2Git\edk2\IntelFrameworkModulePkg\Library\GenericBdsLib):B
> dsLibMatchDevicePaths ( BdsMisc.c
> (F:\GIT\edk2Git\edk2\Vlv2TbltDevicePkg\Override\IntelFrameworkModule
> Pkg\Library\GenericBdsLib):BdsLibMatchDevicePaths (
> ConsoleOption.c
> (F:\GIT\edk2Git\edk2\IntelFrameworkModulePkg\Universal\BdsDxe\BootM
> aint):      if (BdsLibMatchDevicePaths (OutDevicePath, NewDevicePath)) {
> ConsoleOption.c
> (F:\GIT\edk2Git\edk2\IntelFrameworkModulePkg\Universal\BdsDxe\BootM
> aint):      if (BdsLibMatchDevicePaths (InpDevicePath, NewDevicePath)) {
> ConsoleOption.c
> (F:\GIT\edk2Git\edk2\IntelFrameworkModulePkg\Universal\BdsDxe\BootM
> aint):      if (BdsLibMatchDevicePaths (ErrDevicePath, NewDevicePath)) {
> ConsoleOption.c
> (F:\GIT\edk2Git\edk2\IntelFrameworkModulePkg\Universal\BdsDxe\BootM
> aint):    NewConsoleContext->IsActive = BdsLibMatchDevicePaths (
> GenericBdsLib.h
> (F:\GIT\edk2Git\edk2\IntelFrameworkModulePkg\Include\Library):BdsLibM
> atchDevicePaths (
> ---- ConPlatformMatchDevicePaths Matches (4 in 2 files) ---- ConPlatform.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Universal\Console\ConPlatformDxe)
> :ConPlatformMatchDevicePaths (
> ConPlatform.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Universal\Console\ConPlatformDxe):
> Status = ConPlatformMatchDevicePaths (
> ConPlatform.c
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Universal\Console\ConPlatformDxe):
> Status = ConPlatformMatchDevicePaths (
> ConPlatform.h
> (F:\GIT\edk2Git\edk2\MdeModulePkg\Universal\Console\ConPlatformDxe)
> :ConPlatformMatchDevicePaths (
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Wednesday, April 19, 2017 10:43 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Avoid always append
> device path to *Dev
> 
> When TerminalDxe Start() is called multiple times, the old logic
> unconditionally appended the terminal device path candidates to *Dev
> (ConInDev/ConOutDev/ErrOutDev), resulting the volatile storage is full.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
>  .../Universal/Console/TerminalDxe/Terminal.c       | 67
> +++++++++++++++++++---
>  1 file changed, 59 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> index 5d55969..60de2d4 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
> @@ -994,6 +994,49 @@ TerminalDriverBindingStop (  }
> 
>  /**
> +  Compare a device path data structure to that of all the nodes of a
> + second device path instance.
> +
> +  @param  Multi          A pointer to a multi-instance device path data structure.
> +  @param  Single         A pointer to a single-instance device path data
> structure.
> +
> +  @retval TRUE           If the Single is contained within Multi.
> +  @retval FALSE          The Single is not match within Multi.
> +
> +**/
> +BOOLEAN
> +MatchDevicePaths (
> +  IN  EFI_DEVICE_PATH_PROTOCOL  *Multi,
> +  IN  EFI_DEVICE_PATH_PROTOCOL  *Single
> +  )
> +{
> +  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
> +  EFI_DEVICE_PATH_PROTOCOL  *DevicePathInst;
> +  UINTN                     Size;
> +
> +  DevicePath      = Multi;
> +  DevicePathInst  = GetNextDevicePathInstance (&DevicePath, &Size);  //
> + // Search for the match of 'Single' in 'Multi'
> +  //
> +  while (DevicePathInst != NULL) {
> +    //
> +    // If the single device path is found in multiple device paths,
> +    // return success
> +    //
> +    if (CompareMem (Single, DevicePathInst, Size) == 0) {
> +      FreePool (DevicePathInst);
> +      return TRUE;
> +    }
> +
> +    FreePool (DevicePathInst);
> +    DevicePathInst = GetNextDevicePathInstance (&DevicePath, &Size);  }
> +
> +  return FALSE;
> +}
> +
> +/**
>    Update terminal device path in Console Device Environment Variables.
> 
>    @param  VariableName           The Console Device Environment Variable.
> @@ -1018,8 +1061,12 @@ TerminalUpdateConsoleDevVariable (
>    //
>    // Get global variable and its size according to the name given.
>    //
> -  GetEfiGlobalVariable2 (VariableName, (VOID**)&Variable, NULL);
> -  if (Variable == NULL) {
> +  Status = GetEfiGlobalVariable2 (VariableName, (VOID**)&Variable,
> + NULL);  if (Status == EFI_NOT_FOUND) {
> +    Status   = EFI_SUCCESS;
> +    Variable = NULL;
> +  }
> +  if (EFI_ERROR (Status)) {
>      return;
>    }
> 
> @@ -1028,17 +1075,21 @@ TerminalUpdateConsoleDevVariable (
>    //
>    for (TerminalType = 0; TerminalType < ARRAY_SIZE (mTerminalType);
> TerminalType++) {
>      SetTerminalDevicePath (TerminalType, ParentDevicePath,
> &TempDevicePath);
> -    NewVariable = AppendDevicePathInstance (Variable, TempDevicePath);
> -    ASSERT (NewVariable != NULL);
> -    if (Variable != NULL) {
> -      FreePool (Variable);
> -    }
> 
>      if (TempDevicePath != NULL) {
> +      if (!MatchDevicePaths (Variable, TempDevicePath)) {
> +        NewVariable = AppendDevicePathInstance (Variable, TempDevicePath);
> +        if (NewVariable != NULL) {
> +          if (Variable != NULL) {
> +            FreePool (Variable);
> +          }
> +          Variable = NewVariable;
> +        }
> +      }
> +
>        FreePool (TempDevicePath);
>      }
> 
> -    Variable = NewVariable;
>    }
> 
>    VariableSize = GetDevicePathSize (Variable);
> --
> 2.9.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel