public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 1/3] MdeModulePkg/PartitionDxe: Initialize the array after declaration
@ 2017-09-11  6:17 Dandan Bi
  2017-09-11  6:17 ` [PATCH v2 2/3] MdeModulePkg/UdfDxe: " Dandan Bi
  2017-09-11  6:17 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools Dandan Bi
  0 siblings, 2 replies; 7+ messages in thread
From: Dandan Bi @ 2017-09-11  6:17 UTC (permalink / raw)
  To: edk2-devel

Initialize the array DescriptorLBAs[] after declaration to fix
non-constant aggregate initializer warning in VS tool chains.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index 3347b48..7856b5d 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -46,15 +46,22 @@ FindAnchorVolumeDescriptorPointer (
   IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
   OUT  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint
   )
 {
   EFI_STATUS  Status;
-  UINT32      BlockSize = BlockIo->Media->BlockSize;
-  EFI_LBA     EndLBA = BlockIo->Media->LastBlock;
-  EFI_LBA     DescriptorLBAs[] = { 256, EndLBA - 256, EndLBA, 512 };
+  UINT32      BlockSize;
+  EFI_LBA     EndLBA;
+  EFI_LBA     DescriptorLBAs[4];
   UINTN       Index;
 
+  BlockSize = BlockIo->Media->BlockSize;
+  EndLBA = BlockIo->Media->LastBlock;
+  DescriptorLBAs[0] = 256;
+  DescriptorLBAs[1] = EndLBA - 256;
+  DescriptorLBAs[2] = EndLBA;
+  DescriptorLBAs[3] = 512;
+
   for (Index = 0; Index < ARRAY_SIZE (DescriptorLBAs); Index++) {
     Status = DiskIo->ReadDisk (
       DiskIo,
       BlockIo->Media->MediaId,
       MultU64x32 (DescriptorLBAs[Index], BlockSize),
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/3] MdeModulePkg/UdfDxe: Initialize the array after declaration
  2017-09-11  6:17 [PATCH v2 1/3] MdeModulePkg/PartitionDxe: Initialize the array after declaration Dandan Bi
@ 2017-09-11  6:17 ` Dandan Bi
  2017-09-11  6:17 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools Dandan Bi
  1 sibling, 0 replies; 7+ messages in thread
From: Dandan Bi @ 2017-09-11  6:17 UTC (permalink / raw)
  To: edk2-devel

Initialize the array DescriptorLBAs[] after declaration to fix
non-constant aggregate initializer warning in VS tool chains.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 0de9c71..ea3f5fb 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -20,15 +20,22 @@ FindAnchorVolumeDescriptorPointer (
   IN   EFI_DISK_IO_PROTOCOL                  *DiskIo,
   OUT  UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint
   )
 {
   EFI_STATUS  Status;
-  UINT32      BlockSize = BlockIo->Media->BlockSize;
-  EFI_LBA     EndLBA = BlockIo->Media->LastBlock;
-  EFI_LBA     DescriptorLBAs[] = { 256, EndLBA - 256, EndLBA, 512 };
+  UINT32      BlockSize;
+  EFI_LBA     EndLBA;
+  EFI_LBA     DescriptorLBAs[4];
   UINTN       Index;
 
+  BlockSize = BlockIo->Media->BlockSize;
+  EndLBA = BlockIo->Media->LastBlock;
+  DescriptorLBAs[0] = 256;
+  DescriptorLBAs[1] = EndLBA - 256;
+  DescriptorLBAs[2] = EndLBA;
+  DescriptorLBAs[3] = 512;
+
   for (Index = 0; Index < ARRAY_SIZE (DescriptorLBAs); Index++) {
     Status = DiskIo->ReadDisk (
       DiskIo,
       BlockIo->Media->MediaId,
       MultU64x32 (DescriptorLBAs[Index], BlockSize),
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools
  2017-09-11  6:17 [PATCH v2 1/3] MdeModulePkg/PartitionDxe: Initialize the array after declaration Dandan Bi
  2017-09-11  6:17 ` [PATCH v2 2/3] MdeModulePkg/UdfDxe: " Dandan Bi
@ 2017-09-11  6:17 ` Dandan Bi
  2017-09-12  9:39   ` Zeng, Star
  1 sibling, 1 reply; 7+ messages in thread
From: Dandan Bi @ 2017-09-11  6:17 UTC (permalink / raw)
  To: edk2-devel; +Cc: Eric Dong, Paulo Alcantara, Ruiyu Ni, Star Zeng

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;
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools
  2017-09-11  6:17 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools Dandan Bi
@ 2017-09-12  9:39   ` Zeng, Star
  2017-09-12 13:02     ` Paulo Alcantara
  0 siblings, 1 reply; 7+ messages in thread
From: Zeng, Star @ 2017-09-12  9:39 UTC (permalink / raw)
  To: Paulo Alcantara, Bi, Dandan, edk2-devel@lists.01.org
  Cc: Dong, Eric, Ni, Ruiyu, Gao, Liming,
	Laszlo Ersek (lersek@redhat.com), Zeng, Star

Hi Paulo,

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);
           }


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;
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools
  2017-09-12  9:39   ` Zeng, Star
@ 2017-09-12 13:02     ` Paulo Alcantara
  2017-09-13  3:31       ` Zeng, Star
  0 siblings, 1 reply; 7+ messages in thread
From: Paulo Alcantara @ 2017-09-12 13:02 UTC (permalink / raw)
  To: Zeng, Star, Bi, Dandan, edk2-devel@lists.01.org
  Cc: Dong, Eric, Ni, Ruiyu, Gao, Liming,
	Laszlo Ersek (lersek@redhat.com)

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;
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools
  2017-09-12 13:02     ` Paulo Alcantara
@ 2017-09-13  3:31       ` Zeng, Star
  2017-09-13  4:06         ` Paulo Alcantara
  0 siblings, 1 reply; 7+ messages in thread
From: Zeng, Star @ 2017-09-13  3:31 UTC (permalink / raw)
  To: Paulo Alcantara, Bi, Dandan, edk2-devel@lists.01.org
  Cc: Dong, Eric, Ni, Ruiyu, Gao, Liming,
	Laszlo Ersek (lersek@redhat.com), Zeng, Star

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;
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools
  2017-09-13  3:31       ` Zeng, Star
@ 2017-09-13  4:06         ` Paulo Alcantara
  0 siblings, 0 replies; 7+ messages in thread
From: Paulo Alcantara @ 2017-09-13  4:06 UTC (permalink / raw)
  To: Zeng, Star, Bi, Dandan, edk2-devel@lists.01.org
  Cc: Dong, Eric, Ni, Ruiyu, Gao, Liming,
	Laszlo Ersek (lersek@redhat.com)

Hi,

On 13/09/2017 00:31, Zeng, Star wrote:
> Could you help send the patch quickly? As it breaks some platforms build and blocks others' development on that platform, for example Nt32.

Yep. Sorry for the delay. I'll send it shortly.

Thanks!
Paulo

> 
> 
> 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;
>>
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-09-13  4:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-11  6:17 [PATCH v2 1/3] MdeModulePkg/PartitionDxe: Initialize the array after declaration Dandan Bi
2017-09-11  6:17 ` [PATCH v2 2/3] MdeModulePkg/UdfDxe: " Dandan Bi
2017-09-11  6:17 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Add type cast to fix build failure in VS tools Dandan Bi
2017-09-12  9:39   ` Zeng, Star
2017-09-12 13:02     ` Paulo Alcantara
2017-09-13  3:31       ` Zeng, Star
2017-09-13  4:06         ` Paulo Alcantara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox