Re: [PATCH v1 7/7] MdeModulePkg/UdfDxe: Handle dead codes in FileSystemOperations.c

["Zeng, Star" <star.zeng@intel.com>]

On 2018/10/15 12:55, Hao Wu wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1249
> 
> We found potential dead codes within File.c during the code coverage test.
> 
> After manual review, we think the below ones are positive reports:
> 
> A. For function GetAllocationDescriptor():
> Due to the all the calling places for this function, the input parameter
> 'RecordingFlags' can only with value 'LongAdsSequence' or
> 'ShortAdsSequence'.
> 
> So the below code will never be reached:
> 
>    return EFI_DEVICE_ERROR;
> 
> This commit will add "ASSERT (FALSE);" before the above line to indicate
> this.
> 
> B. For function GetAllocationDescriptorLsn():
> Due to the all the calling places for this function, the input parameter
> 'RecordingFlags' can only with value 'LongAdsSequence' or
> 'ShortAdsSequence'.
> 
> So the below code will never be reached:
> 
>    return EFI_UNSUPPORTED;
> 
> This commit will add "ASSERT (FALSE);" before the above line to indicate
> this.
> 
> C. For function SetFileInfo():
> Due to the all the calling places for this function, the input parameter
> 'FileName' will never be a NULL pointer.
> 
> So the below codes will never be reached:
> 
>    } else {
>      FileInfo->FileName[0] = '\0';
>    }
> 
> This commit will add "ASSERT (FALSE);" before the above lines to indicate
> this.

Hao,

Thanks for the patch.

I think we should see what is the expected value for the parameter, but 
not see how caller uses the parameter.
 From this point of view, I think the C case is valid and may be no need 
to change.


Thanks,
Star

> 
> Cc: Paulo Alcantara <paulo@paulo.ac>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>   MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> index 1400846bf1..19acd0554c 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> @@ -738,6 +738,10 @@ GetAllocationDescriptor (
>         );
>     }
>   
> +  //
> +  // Code should never reach here.
> +  //
> +  ASSERT (FALSE);
>     return EFI_DEVICE_ERROR;
>   }
>   
> @@ -784,6 +788,10 @@ GetAllocationDescriptorLsn (
>       return EFI_SUCCESS;
>     }
>   
> +  //
> +  // Code should never reach here.
> +  //
> +  ASSERT (FALSE);
>     return EFI_UNSUPPORTED;
>   }
>   
> @@ -2413,6 +2421,10 @@ SetFileInfo (
>     if (FileName != NULL) {
>       StrCpyS (FileInfo->FileName, StrLen (FileName) + 1, FileName);
>     } else {
> +    //
> +    // Code should never reach here.
> +    //
> +    ASSERT (FALSE);
>       FileInfo->FileName[0] = '\0';
>     }
>   
>