* [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