Re: [PATCH 3/4] ArmPkg/DefaultExceptionHandlerLib: drop BASE variant

[Laszlo Ersek <lersek@redhat.com>]

On 12/20/18 18:31, Ard Biesheuvel wrote:
> Drop the redundant BASE variant, which is no longer used anywhere
> now that DebugAgentSymbolsBaseLib no longer incorporates a vector
> table and exception handling.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/ArmPkg.dsc                                                            |  1 -
>  ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c      | 35 ---------------
>  ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf | 45 --------------------
>  ArmVirtPkg/ArmVirt.dsc.inc                                                   |  1 -
>  4 files changed, 82 deletions(-)
> 
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 5d83c18b143e..d9f9935d70b6 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -141,7 +141,6 @@
>    ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>    ArmPkg/Library/ArmLib/ArmBaseLib.inf
>    ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> -  ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
>    ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
>    ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  
> diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c
> deleted file mode 100644
> index 4a54298b1189..000000000000
> --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerBase.c
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/** @file
> -
> -  Copyright (c) 2012, ARM Ltd. All rights reserved.<BR>
> -
> -  This program and the accompanying materials
> -  are licensed and made available under the terms and conditions of the BSD License
> -  which accompanies this distribution.  The full text of the license may be found at
> -  http://opensource.org/licenses/bsd-license.php
> -
> -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -
> -**/
> -
> -#include <Base.h>
> -
> -/**
> -
> -  @param  FaultAddress         Address to find PE/COFF image for.
> -  @param  ImageBase            Return load address of found image
> -  @param  PeCoffSizeOfHeaders  Return the size of the PE/COFF header for the image that was found
> -
> -  @retval NULL                 FaultAddress not in a loaded PE/COFF image.
> -  @retval                      Path and file name of PE/COFF image.
> -
> -**/
> -CHAR8 *
> -GetImageName (
> -  IN  UINTN  FaultAddress,
> -  OUT UINTN  *ImageBase,
> -  OUT UINTN  *PeCoffSizeOfHeaders
> -  )
> -{
> -  return NULL;
> -}
> diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
> deleted file mode 100644
> index b53a5e89f507..000000000000
> --- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -#/** @file
> -#
> -# Copyright (c) 2012, ARM Ltd. All rights reserved.<BR>
> -#
> -#  This program and the accompanying materials
> -#  are licensed and made available under the terms and conditions of the BSD License
> -#  which accompanies this distribution. The full text of the license may be found at
> -#  http://opensource.org/licenses/bsd-license.php
> -#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> -#
> -#
> -#**/
> -
> -[Defines]
> -  INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = DefaultExceptionHandlerBaseLib
> -  FILE_GUID                      = 3d5261d5-5eb7-4559-98e7-475aa9d0dc42
> -  MODULE_TYPE                    = BASE
> -  VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = DefaultExceptionHandlerLib
> -
> -[Sources.common]
> -  DefaultExceptionHandlerBase.c
> -
> -[Sources.ARM]
> -  Arm/DefaultExceptionHandler.c
> -
> -[Sources.AARCH64]
> -  AArch64/DefaultExceptionHandler.c
> -
> -[Packages]
> -  MdePkg/MdePkg.dec
> -  ArmPkg/ArmPkg.dec
> -
> -[LibraryClasses]
> -  BaseLib
> -  PrintLib
> -  DebugLib
> -  PeCoffGetEntryPointLib
> -  ArmDisassemblerLib
> -  SerialPortLib
> -
> -[Guids]
> -  gEfiDebugImageInfoTableGuid
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 89c2db074711..c47955be940c 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -174,7 +174,6 @@
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>  
>    DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
> -  DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
>    SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf
>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>    PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> 

Can you split the ArmVirtPkg hunk to a separate patch? I think that,
after patch #1, it should be possible to remove the
[LibraryClasses.common.SEC] resolution from "ArmVirt.dsc.inc". Then the
present patch can continue saying "...  no longer used anywhere".

Such a split would be more idiomatic to edk2, and also more faithful to
the current subject line (which says "ArmPkg/DefaultExceptionHandlerLib:
...").

Thanks
Laszlo