Hi Ard,
Some minor comments,
1. Could you please also remove the arguments in related function comments?
2. Please make the code aligned after removing some condition check.
Thanks,
Dandan
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Friday, April 10, 2020 12:10 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>;
> Ard Biesheuvel <ard.biesheu...@arm.com>
> Subject: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore/Image: remove
> unused function arguments
>
> Image.c defines a CoreLoadImageCommon() function that has some
> arguments that are marked OPTIONAL. The function only has a single caller
> living in the same file, and it does not pass any arguments for these
> OPTIONAL arguments.
>
> So let's simplify the code, and remove these arguments and all the transitive
> conditional dependencies on them.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@arm.com>
> ---
>
> NOTE: this patch was generated with the -w option, to suppress indentation
> changes from cluttering the diff.
>
> MdeModulePkg/Core/Dxe/Image/Image.c | 81 ++------------------
> 1 file changed, 7 insertions(+), 74 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index aae2c1eaa516..9fdde87f2df3 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -564,13 +564,10 @@ CoreLoadPeImage (
> IN BOOLEAN BootPolicy,
> IN VOID *Pe32Handle,
> IN LOADED_IMAGE_PRIVATE_DATA *Image,
> - IN EFI_PHYSICAL_ADDRESS DstBuffer OPTIONAL,
> - OUT EFI_PHYSICAL_ADDRESS *EntryPoint OPTIONAL,
> IN UINT32 Attribute
> )
> {
> EFI_STATUS Status;
> - BOOLEAN DstBufAlocated;
> UINTN Size;
>
> ZeroMem (&Image->ImageContext, sizeof (Image->ImageContext)); @@ -
> 619,15 +616,6 @@ CoreLoadPeImage (
> return EFI_UNSUPPORTED;
> }
>
> - //
> - // Allocate memory of the correct memory type aligned on the required
> image boundary
> - //
> - DstBufAlocated = FALSE;
> - if (DstBuffer == 0) {
> - //
> - // Allocate Destination Buffer as caller did not pass it in
> - //
> -
> if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {
> Size = (UINTN)Image->ImageContext.ImageSize + Image-
> >ImageContext.SectionAlignment;
> } else {
> @@ -685,31 +673,6 @@ CoreLoadPeImage (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - DstBufAlocated = TRUE;
> - } else {
> - //
> - // Caller provided the destination buffer
> - //
> -
> - if (Image->ImageContext.RelocationsStripped && (Image-
> >ImageContext.ImageAddress != DstBuffer)) {
> - //
> - // If the image relocations were stripped, and the caller provided a
> - // destination buffer address that does not match the address that the
> - // image is linked at, then the image cannot be loaded.
> - //
> - return EFI_INVALID_PARAMETER;
> - }
> -
> - if (Image->NumberOfPages != 0 &&
> - Image->NumberOfPages <
> - (EFI_SIZE_TO_PAGES ((UINTN)Image->ImageContext.ImageSize +
> Image->ImageContext.SectionAlignment))) {
> - Image->NumberOfPages = EFI_SIZE_TO_PAGES ((UINTN)Image-
> >ImageContext.ImageSize + Image->ImageContext.SectionAlignment);
> - return EFI_BUFFER_TOO_SMALL;
> - }
> -
> - Image->NumberOfPages = EFI_SIZE_TO_PAGES ((UINTN)Image-
> >ImageContext.ImageSize + Image->ImageContext.SectionAlignment);
> - Image->ImageContext.ImageAddress = DstBuffer;
> - }
>
> Image->ImageBasePage = Image->ImageContext.ImageAddress;
> if (!Image->ImageContext.IsTeImage) { @@ -790,13 +753,6 @@
> CoreLoadPeImage (
> }
> }
>
> - //
> - // Fill in the entry point of the image if it is available
> - //
> - if (EntryPoint != NULL) {
> - *EntryPoint = Image->ImageContext.EntryPoint;
> - }
> -
> //
> // Print the load address and the PDB file name if it is available
> //
> @@ -861,11 +817,9 @@ CoreLoadPeImage (
> // Free memory.
> //
>
> - if (DstBufAlocated) {
> CoreFreePages (Image->ImageContext.ImageAddress, Image-
> >NumberOfPages);
> Image->ImageContext.ImageAddress = 0;
> Image->ImageBasePage = 0;
> - }
>
> if (Image->ImageContext.FixupData != NULL) {
> CoreFreePool (Image->ImageContext.FixupData); @@ -920,8 +874,7 @@
> CoreLoadedImageInfo ( STATIC VOID CoreUnloadAndCloseImage (
> - IN LOADED_IMAGE_PRIVATE_DATA *Image,
> - IN BOOLEAN FreePage
> + IN LOADED_IMAGE_PRIVATE_DATA *Image
> )
> {
> EFI_STATUS Status;
> @@ -1047,7 +1000,7 @@ CoreUnloadAndCloseImage (
> //
> // Free the Image from memory
> //
> - if ((Image->ImageBasePage != 0) && FreePage) {
> + if ((Image->ImageBasePage != 0)) {
> CoreFreePages (Image->ImageBasePage, Image->NumberOfPages);
> }
>
> @@ -1122,10 +1075,7 @@ CoreLoadImageCommon (
> IN EFI_DEVICE_PATH_PROTOCOL *FilePath,
> IN VOID *SourceBuffer OPTIONAL,
> IN UINTN SourceSize,
> - IN EFI_PHYSICAL_ADDRESS DstBuffer OPTIONAL,
> - IN OUT UINTN *NumberOfPages OPTIONAL,
> OUT EFI_HANDLE *ImageHandle,
> - OUT EFI_PHYSICAL_ADDRESS *EntryPoint OPTIONAL,
> IN UINT32 Attribute
> )
> {
> @@ -1330,12 +1280,7 @@ CoreLoadImageCommon (
> Image->Info.FilePath = DuplicateDevicePath (FilePath);
> Image->Info.ParentHandle = ParentImageHandle;
>
> -
> - if (NumberOfPages != NULL) {
> - Image->NumberOfPages = *NumberOfPages ;
> - } else {
> Image->NumberOfPages = 0 ;
> - }
>
> //
> // Install the protocol interfaces for this image @@ -1355,20 +1300,11 @@
> CoreLoadImageCommon (
> //
> // Load the image. If EntryPoint is Null, it will not be set.
> //
> - Status = CoreLoadPeImage (BootPolicy, &FHand, Image, DstBuffer,
> EntryPoint, Attribute);
> + Status = CoreLoadPeImage (BootPolicy, &FHand, Image, Attribute);
> if (EFI_ERROR (Status)) {
> - if ((Status == EFI_BUFFER_TOO_SMALL) || (Status ==
> EFI_OUT_OF_RESOURCES)) {
> - if (NumberOfPages != NULL) {
> - *NumberOfPages = Image->NumberOfPages;
> - }
> - }
> goto Done;
> }
>
> - if (NumberOfPages != NULL) {
> - *NumberOfPages = Image->NumberOfPages;
> - }
> -
> //
> // Register the image in the Debug Image Info Table if the attribute is set
> //
> @@ -1448,7 +1384,7 @@ CoreLoadImageCommon (
> //
> if (EFI_ERROR (Status)) {
> if (Image != NULL) {
> - CoreUnloadAndCloseImage (Image, (BOOLEAN)(DstBuffer == 0));
> + CoreUnloadAndCloseImage (Image);
> Image = NULL;
> }
> } else if (EFI_ERROR (SecurityStatus)) { @@ -1524,10 +1460,7 @@
> CoreLoadImage (
> FilePath,
> SourceBuffer,
> SourceSize,
> - (EFI_PHYSICAL_ADDRESS) (UINTN) NULL,
> - NULL,
> ImageHandle,
> - NULL,
> EFI_LOAD_PE_IMAGE_ATTRIBUTE_RUNTIME_REGISTRATION |
> EFI_LOAD_PE_IMAGE_ATTRIBUTE_DEBUG_IMAGE_INFO_TABLE_REGISTRA
> TION
> );
>
> @@ -1744,7 +1677,7 @@ CoreStartImage (
> // unload it
> //
> if (EFI_ERROR (Image->Status) || Image->Type ==
> EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION) {
> - CoreUnloadAndCloseImage (Image, TRUE);
> + CoreUnloadAndCloseImage (Image);
> //
> // ImageHandle may be invalid after the image is unloaded, so use NULL
> handle to record perf log.
> //
> @@ -1809,7 +1742,7 @@ CoreExit (
> //
> // The image has not been started so just free its resources
> //
> - CoreUnloadAndCloseImage (Image, TRUE);
> + CoreUnloadAndCloseImage (Image);
> Status = EFI_SUCCESS;
> goto Done;
> }
> @@ -1911,7 +1844,7 @@ CoreUnloadImage (
> //
> // if the Image was not started or Unloaded O.K. then clean up
> //
> - CoreUnloadAndCloseImage (Image, TRUE);
> + CoreUnloadAndCloseImage (Image);
> }
>
> Done:
> --
> 2.17.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60488): https://edk2.groups.io/g/devel/message/60488
Mute This Topic: https://groups.io/mt/72900049/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
