Re: [edk2-devel] [PATCH] MdeModulePkg: Initialize local variable value before they are used

["Laszlo Ersek" <lersek@redhat.com>]

Hi Liming,

On 03/09/21 02:24, gaoliming wrote:
> This change is to fix the compiler error on GCC49 release build.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
> ---
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c                 | 1 +
>  .../Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c    | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> index e99a812a44..0779f94f9e 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> @@ -1122,6 +1122,7 @@ AhciDmaTransfer (
>  
>    Map   = NULL;
>    PciIo = Instance->PciIo;
> +  Status = EFI_SUCCESS;
>  
>    if (PciIo == NULL) {
>      return EFI_INVALID_PARAMETER;
> diff --git a/MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c b/MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
> index 0c9299c8b0..7822cbf4bb 100644
> --- a/MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
> +++ b/MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
> @@ -117,6 +117,7 @@ CreateBasicVariablePolicy (
>  
>    // Now we've gotta determine the total size of the buffer required for
>    // the VariablePolicy structure.
> +  NameSize  = 0;
>    TotalSize = sizeof( VARIABLE_POLICY_ENTRY );
>    if (Name != NULL) {
>      NameSize = StrnSizeS( Name, MAX_UINT16 );
> 

(1) we have a bugzilla ticket related to this (or more precisely,
overlapping AtaAtapiPassThru):

https://bugzilla.tianocore.org/show_bug.cgi?id=3228

I think mentioning the BZ in the commit message might make sense.

(2) Recently we have used a special comment format for such assignments.
Namely:

  //
  // Set Status to suppress incorrect compiler/analyzer warnings
  //
  Status = EFI_SUCCESS;

Hao already requested that we should document that we only suppress
compiler false positives with these assignments -- they are not needed
functionally. However, I think saying so in the commit message *only* is
not sufficient. I think we should stick with the above code-comment
format (which we've used recently in several places).

Thanks!
Laszlo