Re: [edk2-devel] [PATCH V1 2/2] MdeModulePkg PeiCore: Improve comment semantics

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 11/27/19 5:06 AM, Kubacki, Michael A via Groups.Io wrote:
> Clarifies wording in several PeiCore comments to improve

"Clarify"?

> reading comprehension.
> 
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
>   MdeModulePkg/Core/Pei/FwVol/FwVol.h           |  4 ++--
>   MdeModulePkg/Core/Pei/PeiMain.h               | 11 +++++-----
>   MdeModulePkg/Core/Pei/Dependency/Dependency.c |  4 ++--
>   MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c |  4 ++--
>   MdeModulePkg/Core/Pei/FwVol/FwVol.c           | 23 ++++++++++----------
>   MdeModulePkg/Core/Pei/Memory/MemoryServices.c |  4 ++--
>   6 files changed, 26 insertions(+), 24 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.h b/MdeModulePkg/Core/Pei/FwVol/FwVol.h
> index 263f0d7a56..8aaf84870b 100644
> --- a/MdeModulePkg/Core/Pei/FwVol/FwVol.h
> +++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.h
> @@ -303,9 +303,9 @@ FindFileEx (
>     );
>   
>   /**
> -  Report the information for a new discovered FV in unknown format.
> +  Report the information for a newly discovered FV in an unknown format.
>   
> -  If the EFI_PEI_FIRMWARE_VOLUME_PPI has not been installed for specific FV format, but
> +  If the EFI_PEI_FIRMWARE_VOLUME_PPI has not been installed for a third-party FV format, but
>     the FV in this FV format has been discovered, then the information of this FV
>     will be cached into PEI_CORE_INSTANCE's UnknownFvInfo array.
>     Also a notification would be installed for unknown FV format GUID, if EFI_PEI_FIRMWARE_VOLUME_PPI
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
> index 3f61247a0f..96d6df0485 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -1217,8 +1217,8 @@ PeiFfsGetVolumeInfo (
>     );
>   
>   /**
> -  This routine enable a PEIM to register itself to shadow when PEI Foundation
> -  discovery permanent memory.
> +  This routine enables a PEIM to register itself for shadow when the PEI Foundation
> +  discovers permanent memory.
>   
>     @param FileHandle             File handle of a PEIM.
>   
> @@ -1314,12 +1314,13 @@ ProcessFvFile (
>     );
>   
>   /**
> -  Get instance of PEI_CORE_FV_HANDLE for next volume according to given index.
> +  Gets a PEI_CORE_FV_HANDLE instance for the next volume according to the given index.

"Get"?

>   
> -  This routine also will install FvInfo PPI for FV HOB in PI ways.
> +  This routine also will install an instance of the FvInfo PPI for the FV HOB
> +  as defined in the PI specification.
>   
>     @param Private    Pointer of PEI_CORE_INSTANCE
> -  @param Instance   The index of FV want to be searched.
> +  @param Instance   The index of the FV to search.

Maybe without "The" and trailing dot?

>   
>     @return Instance of PEI_CORE_FV_HANDLE.
>   **/
> diff --git a/MdeModulePkg/Core/Pei/Dependency/Dependency.c b/MdeModulePkg/Core/Pei/Dependency/Dependency.c
> index 9a8353aef2..b53e5f2686 100644
> --- a/MdeModulePkg/Core/Pei/Dependency/Dependency.c
> +++ b/MdeModulePkg/Core/Pei/Dependency/Dependency.c
> @@ -2,8 +2,8 @@
>     PEI Dispatcher Dependency Evaluator
>   
>     This routine evaluates a dependency expression (DEPENDENCY_EXPRESSION) to determine
> -  if a driver can be scheduled for execution.  The criteria for
> -  schedulability is that the dependency expression is satisfied.
> +  if a driver can be scheduled for execution.  The criteria to be scheduled is
> +  that the dependency expression is satisfied.
>   
>   Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>   SPDX-License-Identifier: BSD-2-Clause-Patent
> diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> index c9f2a91264..a18ac47f61 100644
> --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> @@ -1347,8 +1347,8 @@ DepexSatisfied (
>   }
>   
>   /**
> -  This routine enable a PEIM to register itself to shadow when PEI Foundation
> -  discovery permanent memory.
> +  This routine enables a PEIM to register itself for shadow when the PEI Foundation
> +  discovers permanent memory.
>   
>     @param FileHandle             File handle of a PEIM.
>   
> diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> index c21eb9c039..a9fa476846 100644
> --- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> +++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
> @@ -593,7 +593,7 @@ FirmwareVolumeInfoPpiNotifyCallback (
>     }
>   
>     //
> -  // Locate the corresponding FV_PPI according to founded FV's format GUID
> +  // Locate the corresponding FV_PPI according to the format GUID of the FV found
>     //
>     Status = PeiServicesLocatePpi (
>                &FvInfo2Ppi.FvFormat,
> @@ -1533,7 +1533,7 @@ ProcessFvFile (
>         );
>   
>       //
> -    // Inform the extracted FvImage to FV HOB consumer phase, i.e. DXE phase
> +    // Expose the extracted FvImage to the FV HOB consumer phase, i.e. DXE phase
>       //
>       BuildFvHob (
>         (EFI_PHYSICAL_ADDRESS) (UINTN) FvHeader,
> @@ -2087,12 +2087,13 @@ FvHandleToCoreHandle (
>   }
>   
>   /**
> -  Get instance of PEI_CORE_FV_HANDLE for next volume according to given index.
> +  Gets a PEI_CORE_FV_HANDLE instance for the next volume according to the given index.

"Get"?

>   
> -  This routine also will install FvInfo PPI for FV HOB in PI ways.
> +  This routine also will install an instance of the FvInfo PPI for the FV HOB
> +  as defined in the PI specification.
>   
>     @param Private    Pointer of PEI_CORE_INSTANCE
> -  @param Instance   The index of FV want to be searched.
> +  @param Instance   The index of the FV to search.

Without "The"/trailing dot?

>   
>     @return Instance of PEI_CORE_FV_HANDLE.
>   **/
> @@ -2185,13 +2186,13 @@ PeiReinitializeFv (
>   }
>   
>   /**
> -  Report the information for a new discovered FV in unknown third-party format.
> +  Report the information for a newly discovered FV in an unknown format.
>   
> -  If the EFI_PEI_FIRMWARE_VOLUME_PPI has not been installed for third-party FV format, but
> -  the FV in this format has been discovered, then this FV's information will be cached into
> -  PEI_CORE_INSTANCE's UnknownFvInfo array.
> -  Also a notification would be installed for unknown third-party FV format guid, if EFI_PEI_FIRMWARE_VOLUME_PPI
> -  is installed later by platform's PEIM, the original unknown third-party FV will be processed by
> +  If the EFI_PEI_FIRMWARE_VOLUME_PPI has not been installed for a third-party FV format, but
> +  the FV in this FV format has been discovered, then the information of this FV

Maybe "in this FV format" is redundant?

> +  will be cached into PEI_CORE_INSTANCE's UnknownFvInfo array.
> +  Also a notification would be installed for unknown FV format GUID, if EFI_PEI_FIRMWARE_VOLUME_PPI
> +  is installed later by platform's PEIM, the original unknown FV will be processed by
>     using new installed EFI_PEI_FIRMWARE_VOLUME_PPI.
>   
>     @param PrivateData  Point to instance of PEI_CORE_INSTANCE
> diff --git a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
> index 838a003baa..e713e6811a 100644
> --- a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
> +++ b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
> @@ -759,7 +759,7 @@ PeiFreePages (
>   /**
>   
>     Pool allocation service. Before permanent memory is discovered, the pool will
> -  be allocated the heap in the temporary memory. Generally, the size of heap in temporary
> +  be allocated in the heap in temporary memory. Generally, the size of the heap in temporary
>     memory does not exceed to 64K, so the biggest pool size could be allocated is
>     64K.
>   
> @@ -789,7 +789,7 @@ PeiAllocatePool (
>     //
>   
>     //
> -  // Generally, the size of heap in temporary memory does not exceed to 64K,
> +  // Generally, the size of heap in temporary memory does not exceed 64K,
>     // HobLength is multiples of 8 bytes, so the maximum size of pool is 0xFFF8 - sizeof (EFI_HOB_MEMORY_POOL)
>     //
>     if (Size > (0xFFF8 - sizeof (EFI_HOB_MEMORY_POOL))) {
>