[edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore/Image: remove unused function arguments
Dandan Bi
dandan.bi at intel.com
Mon Jun 1 08:13:27 UTC 2020
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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Friday, April 10, 2020 12:10 AM
> To: devel at edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A <hao.a.wu at intel.com>;
> Ard Biesheuvel <ard.biesheuvel at 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.biesheuvel at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list