Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools

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

Could you help send the patch quickly? As it breaks some platforms build and blocks others' development on that platform, for example Nt32.


Thanks,
Star
-----Original Message-----
From: Paulo Alcantara [mailto:pcacjr@zytor.com] 
Sent: Tuesday, September 12, 2017 9:03 PM
To: Zeng, Star <star.zeng@intel.com>; Bi, Dandan <dandan.bi@intel.com>; edk2-devel@lists.01.org
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo Ersek (lersek@redhat.com) <lersek@redhat.com>
Subject: Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools

Hi,

On 9/12/2017 6:39 AM, Zeng, Star wrote:
> There is change(type cast to INT64) below in this patch. After check, we found the " if (Offset < 0) " should be always false comparison as Offset is UINT64 type.
> I have suggested Dandan to remove this change(type case to INT64) at v3 patch series https://lists.01.org/pipermail/edk2-devel/2017-September/014523.html.
> Could you help check and fix the code appropriately?
>             if (Offset < 0) {
> -            Offset = -(Offset);
> +            Offset = - (INT64) (Offset);
>             }

Oh, nice catch! I'll send a patch that fixes it and do some sanity checks later.

Thank you all! Really appreciate it.

Paulo

> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Bi, Dandan
> Sent: Monday, September 11, 2017 2:17 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Paulo Alcantara 
> <pcacjr@zytor.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star 
> <star.zeng@intel.com>
> Subject: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix 
> build failure in VS tools
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>   .../Universal/Disk/UdfDxe/FileSystemOperations.c       | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c 
> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> index ea3f5fb..bf33ae4 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> @@ -477,11 +477,11 @@ DuplicateFid (
>     OUT  UDF_FILE_IDENTIFIER_DESCRIPTOR  **NewFileIdentifierDesc
>     )
>   {
>     *NewFileIdentifierDesc =
>       (UDF_FILE_IDENTIFIER_DESCRIPTOR *)AllocateCopyPool (
> -      GetFidDescriptorLength (FileIdentifierDesc), FileIdentifierDesc);
> +      (UINTN) GetFidDescriptorLength (FileIdentifierDesc), 
> + FileIdentifierDesc);
>   }
>   
>   //
>   // Duplicate either a given File Entry or a given Extended File Entry.
>   //
> @@ -814,20 +814,20 @@ GetAedAdsData (
>     }
>   
>     //
>     // Allocate buffer to read in AED's data.
>     //
> -  *Data = AllocatePool (*Length);
> +  *Data = AllocatePool ((UINTN) (*Length));
>     if (*Data == NULL) {
>       return EFI_OUT_OF_RESOURCES;
>     }
>   
>     return DiskIo->ReadDisk (
>       DiskIo,
>       BlockIo->Media->MediaId,
>       Offset,
> -    *Length,
> +    (UINTN) (*Length),
>       *Data
>       );
>   }
>   
>   //
> @@ -849,11 +849,11 @@ GrowUpBufferToNextAd (
>       *Buffer = AllocatePool (ExtentLength);
>       if (*Buffer == NULL) {
>         return EFI_OUT_OF_RESOURCES;
>       }
>     } else {
> -    *Buffer = ReallocatePool (Length, Length + ExtentLength, *Buffer);
> +    *Buffer = ReallocatePool ((UINTN) Length, (UINTN) (Length + 
> + ExtentLength), *Buffer);
>       if (*Buffer == NULL) {
>         return EFI_OUT_OF_RESOURCES;
>       }
>     }
>   
> @@ -938,29 +938,29 @@ ReadFile (
>         ReadFileInfo->ReadLength = Length;
>       } else if (ReadFileInfo->Flags == READ_FILE_ALLOCATE_AND_READ) {
>         //
>         // Allocate buffer for starting read data.
>         //
> -      ReadFileInfo->FileData = AllocatePool (Length);
> +      ReadFileInfo->FileData = AllocatePool ((UINTN) Length);
>         if (ReadFileInfo->FileData == NULL) {
>           return EFI_OUT_OF_RESOURCES;
>         }
>   
>         //
>         // Read all inline data into ReadFileInfo->FileData
>         //
> -      CopyMem (ReadFileInfo->FileData, Data, Length);
> +      CopyMem (ReadFileInfo->FileData, Data, (UINTN) Length);
>         ReadFileInfo->ReadLength = Length;
>       } else if (ReadFileInfo->Flags == READ_FILE_SEEK_AND_READ) {
>         //
>         // If FilePosition is non-zero, seek file to FilePosition, read
>         // FileDataSize bytes and then updates FilePosition.
>         //
>         CopyMem (
>           ReadFileInfo->FileData,
>           (VOID *)((UINT8 *)Data + ReadFileInfo->FilePosition),
> -        ReadFileInfo->FileDataSize
> +        (UINTN) ReadFileInfo->FileDataSize
>           );
>   
>         ReadFileInfo->FilePosition += ReadFileInfo->FileDataSize;
>       } else {
>         ASSERT (FALSE);
> @@ -1081,11 +1081,11 @@ ReadFile (
>           }
>   
>           if (FilePosition + ExtentLength > ReadFileInfo->FilePosition) {
>             Offset = ReadFileInfo->FilePosition - FilePosition;
>             if (Offset < 0) {
> -            Offset = -(Offset);
> +            Offset = - (INT64) (Offset);
>             }
>           } else {
>             Offset = 0;
>           }
>   
> @@ -1109,11 +1109,11 @@ ReadFile (
>           //
>           Status = DiskIo->ReadDisk (
>             DiskIo,
>             BlockIo->Media->MediaId,
>             Offset + MultU64x32 (Lsn, LogicalBlockSize),
> -          DataLength,
> +          (UINTN) DataLength,
>             (VOID *)((UINT8 *)ReadFileInfo->FileData +
>                      DataOffset)
>             );
>           if (EFI_ERROR (Status)) {
>             goto Error_Read_Disk_Blk;
>