* [PATCH 1/7] MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref
2017-09-15 4:57 [PATCH 0/7] MdeModulePkg/Udf: Code refinements Hao Wu
@ 2017-09-15 4:57 ` Hao Wu
2017-09-15 4:57 ` [PATCH 2/7] MdeModulePkg/UdfDxe: Fix operands of different size in bitwise OP Hao Wu
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hao Wu @ 2017-09-15 4:57 UTC (permalink / raw)
To: edk2-devel; +Cc: Hao Wu, Paulo Alcantara, Ruiyu Ni, Star Zeng, Eric Dong
Case 1 - Within DuplicateFid() & DuplicateFe():
The call to AllocateCopyPool() may return NULL.
Add ASSERTs as checks.
Case 2 - Within UdfRead():
Add ASSERT to ensure 'NewFileEntryData' returned from FindFileEntry()
will not be NULL pointer.
Case 3 - Within GetAllocationDescriptorLsn():
The return value of 'GetPdFromLongAd (Volume, ParentIcb)' may be NULL,
and it will be passed into function GetShortAdLsn() which will
dereference it.
Add ASSERT in GetShortAdLsn() as check.
Case 4 - Within ReadFile():
Add ASSERT to ensure 'Data' returned from GetAedAdsData() will not be NULL
pointer.
Case 5 - Within InternalFindFile():
If both 'Parent->FileIdentifierDesc' and 'Icb' are NULL, then possible
NULL pointer dereference will happen in ReadDirectoryEntry().
Add additional check to resolve.
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 1 +
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 01361141bb..82db75475b 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -427,6 +427,7 @@ UdfRead (
if (EFI_ERROR (Status)) {
goto Error_Find_Fe;
}
+ ASSERT (NewFileEntryData != NULL);
if (IS_FE_SYMLINK (NewFileEntryData)) {
Status = ResolveSymlink (
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 4609580b30..02a73a9eb9 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -297,6 +297,8 @@ GetShortAdLsn (
IN UDF_SHORT_ALLOCATION_DESCRIPTOR *ShortAd
)
{
+ ASSERT (PartitionDesc != NULL);
+
return (UINT64)PartitionDesc->PartitionStartingLocation +
ShortAd->ExtentPosition;
}
@@ -480,6 +482,8 @@ DuplicateFid (
*NewFileIdentifierDesc =
(UDF_FILE_IDENTIFIER_DESCRIPTOR *)AllocateCopyPool (
(UINTN) GetFidDescriptorLength (FileIdentifierDesc), FileIdentifierDesc);
+
+ ASSERT (*NewFileIdentifierDesc != NULL);
}
//
@@ -494,6 +498,8 @@ DuplicateFe (
)
{
*NewFileEntry = AllocateCopyPool (Volume->FileEntrySize, FileEntry);
+
+ ASSERT (*NewFileEntry != NULL);
}
//
@@ -1028,6 +1034,7 @@ ReadFile (
if (EFI_ERROR (Status)) {
goto Error_Get_Aed;
}
+ ASSERT (Data != NULL);
AdOffset = 0;
continue;
@@ -1209,6 +1216,13 @@ InternalFindFile (
VOID *CompareFileEntry;
//
+ // Check if both Parent->FileIdentifierDesc and Icb are NULL.
+ //
+ if ((Parent->FileIdentifierDesc == NULL) && (Icb == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
// Check if parent file is really directory.
//
if (!IS_FE_DIRECTORY (Parent->FileEntry)) {
@@ -1220,6 +1234,10 @@ InternalFindFile (
// FE/EFE and FID descriptors.
//
if (StrCmp (FileName, L".") == 0) {
+ if (Parent->FileIdentifierDesc == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
DuplicateFe (BlockIo, Volume, Parent->FileEntry, &File->FileEntry);
DuplicateFid (Parent->FileIdentifierDesc, &File->FileIdentifierDesc);
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/7] MdeModulePkg/UdfDxe: Fix operands of different size in bitwise OP
2017-09-15 4:57 [PATCH 0/7] MdeModulePkg/Udf: Code refinements Hao Wu
2017-09-15 4:57 ` [PATCH 1/7] MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref Hao Wu
@ 2017-09-15 4:57 ` Hao Wu
2017-09-15 4:57 ` [PATCH 3/7] MdeModulePkg/UdfDxe: Use compare operator for non-boolean comparisons Hao Wu
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hao Wu @ 2017-09-15 4:57 UTC (permalink / raw)
To: edk2-devel; +Cc: Hao Wu, Paulo Alcantara, Ruiyu Ni, Star Zeng, Eric Dong
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 2 +-
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 82db75475b..4c2cf67fa3 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -806,7 +806,7 @@ UdfGetInfo (
}
if (Index < 128) {
- *String |= *(UINT8 *)(OstaCompressed + Index);
+ *String |= (CHAR16)(*(UINT8 *)(OstaCompressed + Index));
}
//
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 02a73a9eb9..f63e7e660b 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -1782,7 +1782,7 @@ GetFileNameFromFid (
}
if (Index < Length) {
- *FileName |= OstaCompressed[Index];
+ *FileName |= (CHAR16)(OstaCompressed[Index]);
}
FileName++;
@@ -1918,7 +1918,7 @@ ResolveSymlink (
}
if (Index < Length) {
- *C |= *(UINT8 *)((UINT8 *)PathComp->ComponentIdentifier + Index);
+ *C |= (CHAR16)(*(UINT8 *)((UINT8 *)PathComp->ComponentIdentifier + Index));
}
C++;
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/7] MdeModulePkg/UdfDxe: Use compare operator for non-boolean comparisons
2017-09-15 4:57 [PATCH 0/7] MdeModulePkg/Udf: Code refinements Hao Wu
2017-09-15 4:57 ` [PATCH 1/7] MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref Hao Wu
2017-09-15 4:57 ` [PATCH 2/7] MdeModulePkg/UdfDxe: Fix operands of different size in bitwise OP Hao Wu
@ 2017-09-15 4:57 ` Hao Wu
2017-09-15 4:57 ` [PATCH 4/7] MdeModulePkg/Udf: Refine function description comments Hao Wu
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hao Wu @ 2017-09-15 4:57 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Paulo Alcantara, Ruiyu Ni, Star Zeng, Eric Dong,
Dandan Bi
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@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 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index f63e7e660b..7c83830ec0 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -1255,7 +1255,7 @@ InternalFindFile (
BlockIo,
DiskIo,
Volume,
- Parent->FileIdentifierDesc ?
+ (Parent->FileIdentifierDesc != NULL) ?
&Parent->FileIdentifierDesc->Icb :
Icb,
Parent->FileEntry,
@@ -2117,7 +2117,7 @@ SetFileInfo (
//
// Calculate the needed size for the EFI_FILE_INFO structure.
//
- FileInfoLength = sizeof (EFI_FILE_INFO) + (FileName ?
+ FileInfoLength = sizeof (EFI_FILE_INFO) + ((FileName != NULL) ?
StrSize (FileName) :
sizeof (CHAR16));
if (*BufferSize < FileInfoLength) {
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/7] MdeModulePkg/Udf: Refine function description comments
2017-09-15 4:57 [PATCH 0/7] MdeModulePkg/Udf: Code refinements Hao Wu
` (2 preceding siblings ...)
2017-09-15 4:57 ` [PATCH 3/7] MdeModulePkg/UdfDxe: Use compare operator for non-boolean comparisons Hao Wu
@ 2017-09-15 4:57 ` Hao Wu
2017-09-15 4:57 ` [PATCH 5/7] MdeModulePkg/UdfDxe: Avoid short (single character) variable name Hao Wu
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hao Wu @ 2017-09-15 4:57 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Paulo Alcantara, Ruiyu Ni, Star Zeng, Eric Dong,
Dandan Bi
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 12 +
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 2 +-
MdeModulePkg/Universal/Disk/UdfDxe/FileName.c | 27 +-
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 375 +++++++++++++++-----
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 21 +-
5 files changed, 345 insertions(+), 92 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index 7856b5dfc1..c566bfc594 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -40,6 +40,18 @@ UDF_DEVICE_PATH gUdfDevicePath = {
}
};
+/**
+ Find the anchor volume descriptor pointer.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[out] AnchorPoint Anchor volume descriptor pointer.
+
+ @retval EFI_SUCCESS Anchor volume descriptor pointer found.
+ @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
+ @retval other Anchor volume descriptor pointer not found.
+
+**/
EFI_STATUS
FindAnchorVolumeDescriptorPointer (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 4c2cf67fa3..625f2c5637 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -861,7 +861,7 @@ UdfGetInfo (
/**
Set information about a file.
- @param File Protocol instance pointer.
+ @param This Protocol instance pointer.
@param InformationType Type of information in Buffer.
@param BufferSize Size of buffer.
@param Buffer The data to write.
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileName.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileName.c
index f73793320d..36551a4dba 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileName.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileName.c
@@ -14,6 +14,14 @@
#include "Udf.h"
+/**
+ Trim the leading and trailing spaces for a give Unicode string.
+
+ @param[in] String The Unicode string to trim.
+
+ @return A pointer to the trimmed string.
+
+**/
CHAR16 *
TrimString (
IN CHAR16 *String
@@ -35,6 +43,14 @@ TrimString (
return String;
}
+/**
+ Replace the content of a Unicode string with the content of another Unicode
+ string.
+
+ @param[in] Destination A pointer to a Unicode string.
+ @param[in] Source A pointer to a Unicode string.
+
+**/
VOID
ReplaceLeft (
IN CHAR16 *Destination,
@@ -49,6 +65,15 @@ ReplaceLeft (
}
}
+/**
+ Remove one or more consecutive backslashes starting from the second character
+ of a given Unicode string.
+
+ @param[in] String A pointer to a Unicode string.
+
+ @return A pointer to the modified string.
+
+**/
CHAR16 *
ExcludeTrailingBackslashes (
IN CHAR16 *String
@@ -85,7 +110,7 @@ Exit:
@param[in] FileName Filename.
- @retval @p FileName Filename mangled.
+ @retval The mangled Filename.
**/
CHAR16 *
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 7c83830ec0..8fcc508c41 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -14,6 +14,18 @@
#include "Udf.h"
+/**
+ Find the anchor volume descriptor pointer.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[out] AnchorPoint Anchor volume descriptor pointer.
+
+ @retval EFI_SUCCESS Anchor volume descriptor pointer found.
+ @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
+ @retval other Anchor volume descriptor pointer not found.
+
+**/
EFI_STATUS
FindAnchorVolumeDescriptorPointer (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -58,6 +70,22 @@ FindAnchorVolumeDescriptorPointer (
return EFI_VOLUME_CORRUPTED;
}
+/**
+ Save the content of Logical Volume Descriptors and Partitions Descriptors in
+ memory.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[in] AnchorPoint Anchor volume descriptor pointer.
+ @param[out] Volume UDF volume information structure.
+
+ @retval EFI_SUCCESS The descriptors were saved.
+ @retval EFI_OUT_OF_RESOURCES The descriptors were not saved due to lack of
+ resources.
+ @retval other The descriptors were not saved due to
+ ReadDisk error.
+
+**/
EFI_STATUS
StartMainVolumeDescriptorSequence (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -211,11 +239,17 @@ Error_Alloc_Pds:
return Status;
}
-//
-// Return a Partition Descriptor given a Long Allocation Descriptor. This is
-// necessary to calculate the right extent (LongAd) offset which is added up
-// with partition's starting location.
-//
+/**
+ Return a Partition Descriptor given a Long Allocation Descriptor. This is
+ necessary to calculate the right extent (LongAd) offset which is added up
+ with partition's starting location.
+
+ @param[in] Volume Volume information pointer.
+ @param[in] LongAd Long Allocation Descriptor pointer.
+
+ @return A pointer to a Partition Descriptor.
+
+**/
UDF_PARTITION_DESCRIPTOR *
GetPdFromLongAd (
IN UDF_VOLUME_INFO *Volume,
@@ -270,9 +304,15 @@ GetPdFromLongAd (
return NULL;
}
-//
-// Return logical sector number of a given Long Allocation Descriptor.
-//
+/**
+ Return logical sector number of a given Long Allocation Descriptor.
+
+ @param[in] Volume Volume information pointer.
+ @param[in] LongAd Long Allocation Descriptor pointer.
+
+ @return The logical sector number of a given Long Allocation Descriptor.
+
+**/
UINT64
GetLongAdLsn (
IN UDF_VOLUME_INFO *Volume,
@@ -288,9 +328,15 @@ GetLongAdLsn (
LongAd->ExtentLocation.LogicalBlockNumber;
}
-//
-// Return logical sector number of a given Short Allocation Descriptor.
-//
+/**
+ Return logical sector number of a given Short Allocation Descriptor.
+
+ @param[in] PartitionDesc Partition Descriptor pointer.
+ @param[in] ShortAd Short Allocation Descriptor pointer.
+
+ @return The logical sector number of a given Short Allocation Descriptor.
+
+**/
UINT64
GetShortAdLsn (
IN UDF_PARTITION_DESCRIPTOR *PartitionDesc,
@@ -303,12 +349,23 @@ GetShortAdLsn (
ShortAd->ExtentPosition;
}
-//
-// Find File Set Descriptor of a given Logical Volume Descriptor.
-//
-// The found FSD will contain the extent (LogicalVolumeContentsUse) where our
-// root directory is.
-//
+/**
+ Find File Set Descriptor of a given Logical Volume Descriptor.
+
+ The found FSD will contain the extent (LogicalVolumeContentsUse) where our
+ root directory is.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[in] Volume Volume information pointer.
+ @param[in] LogicalVolDescNum Index of Logical Volume Descriptor
+ @param[out] FileSetDesc File Set Descriptor pointer.
+
+ @retval EFI_SUCCESS File Set Descriptor pointer found.
+ @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
+ @retval other File Set Descriptor pointer not found.
+
+**/
EFI_STATUS
FindFileSetDescriptor (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -349,9 +406,20 @@ FindFileSetDescriptor (
return EFI_SUCCESS;
}
-//
-// Get all File Set Descriptors for each Logical Volume Descriptor.
-//
+/**
+ Get all File Set Descriptors for each Logical Volume Descriptor.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[in, out] Volume Volume information pointer.
+
+ @retval EFI_SUCCESS File Set Descriptors were got.
+ @retval EFI_OUT_OF_RESOURCES File Set Descriptors were not got due to lack
+ of resources.
+ @retval other Error occured when finding File Set
+ Descriptor in Logical Volume Descriptor.
+
+**/
EFI_STATUS
GetFileSetDescriptors (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -414,9 +482,17 @@ Error_Alloc_Fsd:
return Status;
}
-//
-// Read Volume and File Structure on an UDF file system.
-//
+/**
+ Read Volume and File Structure on an UDF file system.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[out] Volume Volume information pointer.
+
+ @retval EFI_SUCCESS Volume and File Structure were read.
+ @retval other Volume and File Structure were not read.
+
+**/
EFI_STATUS
ReadVolumeFileStructure (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -455,9 +531,14 @@ ReadVolumeFileStructure (
return Status;
}
-//
-// Calculate length of a given File Identifier Descriptor.
-//
+/**
+ Calculate length of a given File Identifier Descriptor.
+
+ @param[in] FileIdentifierDesc File Identifier Descriptor pointer.
+
+ @return The length of a given File Identifier Descriptor.
+
+**/
UINT64
GetFidDescriptorLength (
IN UDF_FILE_IDENTIFIER_DESCRIPTOR *FileIdentifierDesc
@@ -470,9 +551,13 @@ GetFidDescriptorLength (
);
}
-//
-// Duplicate a given File Identifier Descriptor.
-//
+/**
+ Duplicate a given File Identifier Descriptor.
+
+ @param[in] FileIdentifierDesc File Identifier Descriptor pointer.
+ @param[out] NewFileIdentifierDesc The duplicated File Identifier Descriptor.
+
+**/
VOID
DuplicateFid (
IN UDF_FILE_IDENTIFIER_DESCRIPTOR *FileIdentifierDesc,
@@ -486,9 +571,15 @@ DuplicateFid (
ASSERT (*NewFileIdentifierDesc != NULL);
}
-//
-// Duplicate either a given File Entry or a given Extended File Entry.
-//
+/**
+ Duplicate either a given File Entry or a given Extended File Entry.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] Volume Volume information pointer.
+ @param[in] FileEntry (Extended) File Entry pointer.
+ @param[out] NewFileEntry The duplicated (Extended) File Entry.
+
+**/
VOID
DuplicateFe (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -502,15 +593,21 @@ DuplicateFe (
ASSERT (*NewFileEntry != NULL);
}
-//
-// Get raw data + length of a given File Entry or Extended File Entry.
-//
-// The file's recorded data can contain either real file content (inline) or
-// a sequence of extents (or Allocation Descriptors) which tells where file's
-// content is stored in.
-//
-// NOTE: The FE/EFE can be thought it was an inode.
-//
+/**
+ Get raw data + length of a given File Entry or Extended File Entry.
+
+ The file's recorded data can contain either real file content (inline) or
+ a sequence of extents (or Allocation Descriptors) which tells where file's
+ content is stored in.
+
+ NOTE: The FE/EFE can be thought it was an inode.
+
+ @param[in] FileEntryData (Extended) File Entry pointer.
+ @param[out] Data Buffer contains the raw data of a given
+ (Extended) File Entry.
+ @param[out] Length Length of the data in Buffer.
+
+**/
VOID
GetFileEntryData (
IN VOID *FileEntryData,
@@ -536,9 +633,15 @@ GetFileEntryData (
}
}
-//
-// Get Allocation Descriptors' data information from a given FE/EFE.
-//
+/**
+ Get Allocation Descriptors' data information from a given FE/EFE.
+
+ @param[in] FileEntryData (Extended) File Entry pointer.
+ @param[out] AdsData Buffer contains the Allocation Descriptors'
+ data from a given FE/EFE.
+ @param[out] Length Length of the data in AdsData.
+
+**/
VOID
GetAdsInformation (
IN VOID *FileEntryData,
@@ -564,9 +667,18 @@ GetAdsInformation (
}
}
-//
-// Read next Long Allocation Descriptor from a given file's data.
-//
+/**
+ Read next Long Allocation Descriptor from a given file's data.
+
+ @param[in] Data File's data pointer.
+ @param[in,out] Offset Starting offset of the File's data to read.
+ @param[in] Length Length of the data to read.
+ @param[out] FoundLongAd Long Allocation Descriptor pointer.
+
+ @retval EFI_SUCCESS A Long Allocation Descriptor was found.
+ @retval EFI_DEVICE_ERROR No more Long Allocation Descriptors.
+
+**/
EFI_STATUS
GetLongAdFromAds (
IN VOID *Data,
@@ -611,9 +723,18 @@ GetLongAdFromAds (
return EFI_SUCCESS;
}
-//
-// Read next Short Allocation Descriptor from a given file's data.
-//
+/**
+ Read next Short Allocation Descriptor from a given file's data.
+
+ @param[in] Data File's data pointer.
+ @param[in,out] Offset Starting offset of the File's data to read.
+ @param[in] Length Length of the data to read.
+ @param[out] FoundShortAd Short Allocation Descriptor pointer.
+
+ @retval EFI_SUCCESS A Short Allocation Descriptor was found.
+ @retval EFI_DEVICE_ERROR No more Short Allocation Descriptors.
+
+**/
EFI_STATUS
GetShortAdFromAds (
IN VOID *Data,
@@ -658,10 +779,21 @@ GetShortAdFromAds (
return EFI_SUCCESS;
}
-//
-// Get either a Short Allocation Descriptor or a Long Allocation Descriptor from
-// file's data.
-//
+/**
+ Get either a Short Allocation Descriptor or a Long Allocation Descriptor from
+ file's data.
+
+ @param[in] RecordingFlags Flag to indicate the type of descriptor.
+ @param[in] Data File's data pointer.
+ @param[in,out] Offset Starting offset of the File's data to read.
+ @param[in] Length Length of the data to read.
+ @param[out] FoundAd Allocation Descriptor pointer.
+
+ @retval EFI_SUCCESS A Short Allocation Descriptor was found.
+ @retval EFI_DEVICE_ERROR No more Allocation Descriptors.
+ Invalid type of descriptor was given.
+
+**/
EFI_STATUS
GetAllocationDescriptor (
IN UDF_FE_RECORDING_FLAGS RecordingFlags,
@@ -690,9 +822,17 @@ GetAllocationDescriptor (
return EFI_DEVICE_ERROR;
}
-//
-// Return logical sector number of either Short or Long Allocation Descriptor.
-//
+/**
+ Return logical sector number of either Short or Long Allocation Descriptor.
+
+ @param[in] RecordingFlags Flag to indicate the type of descriptor.
+ @param[in] Volume Volume information pointer.
+ @param[in] ParentIcb Long Allocation Descriptor pointer.
+ @param[in] Ad Allocation Descriptor pointer.
+
+ @return The logical sector number of the given Allocation Descriptor.
+
+**/
UINT64
GetAllocationDescriptorLsn (
IN UDF_FE_RECORDING_FLAGS RecordingFlags,
@@ -713,9 +853,27 @@ GetAllocationDescriptorLsn (
return 0;
}
-//
-// Return offset + length of a given indirect Allocation Descriptor (AED).
-//
+/**
+ Return offset + length of a given indirect Allocation Descriptor (AED).
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[in] Volume Volume information pointer.
+ @param[in] ParentIcb Long Allocation Descriptor pointer.
+ @param[in] RecordingFlags Flag to indicate the type of descriptor.
+ @param[in] Ad Allocation Descriptor pointer.
+ @param[out] Offset Offset of a given indirect Allocation
+ Descriptor.
+ @param[out] Length Length of a given indirect Allocation
+ Descriptor.
+
+ @retval EFI_SUCCESS The offset and length were returned.
+ @retval EFI_OUT_OF_RESOURCES The offset and length were not returned due
+ to lack of resources.
+ @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
+ @retval other The offset and length were not returned.
+
+**/
EFI_STATUS
GetAedAdsOffset (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -784,9 +942,25 @@ Exit:
return Status;
}
-//
-// Read Allocation Extent Descriptor into memory.
-//
+/**
+ Read Allocation Extent Descriptor into memory.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[in] Volume Volume information pointer.
+ @param[in] ParentIcb Long Allocation Descriptor pointer.
+ @param[in] RecordingFlags Flag to indicate the type of descriptor.
+ @param[in] Ad Allocation Descriptor pointer.
+ @param[out] Data Buffer that contains the Allocation Extent
+ Descriptor.
+ @param[out] Length Length of Data.
+
+ @retval EFI_SUCCESS The Allocation Extent Descriptor was read.
+ @retval EFI_OUT_OF_RESOURCES The Allocation Extent Descriptor was not read
+ due to lack of resources.
+ @retval other Fail to read the disk.
+
+**/
EFI_STATUS
GetAedAdsData (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -836,9 +1010,19 @@ GetAedAdsData (
);
}
-//
-// Function used to serialise reads of Allocation Descriptors.
-//
+/**
+ Function used to serialise reads of Allocation Descriptors.
+
+ @param[in] RecordingFlags Flag to indicate the type of descriptor.
+ @param[in] Ad Allocation Descriptor pointer.
+ @param[in, out] Buffer Buffer to hold the next Allocation Descriptor.
+ @param[in] Length Length of Buffer.
+
+ @retval EFI_SUCCESS Buffer was grown to hold the next Allocation
+ Descriptor.
+ @retval EFI_OUT_OF_RESOURCES Buffer was not grown due to lack of resources.
+
+**/
EFI_STATUS
GrowUpBufferToNextAd (
IN UDF_FE_RECORDING_FLAGS RecordingFlags,
@@ -866,9 +1050,26 @@ GrowUpBufferToNextAd (
return EFI_SUCCESS;
}
-//
-// Read data or size of either a File Entry or an Extended File Entry.
-//
+/**
+ Read data or size of either a File Entry or an Extended File Entry.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[in] Volume Volume information pointer.
+ @param[in] ParentIcb Long Allocation Descriptor pointer.
+ @param[in] FileEntryData FE/EFE structure pointer.
+ @param[in, out] ReadFileInfo Read file information pointer.
+
+ @retval EFI_SUCCESS Data or size of a FE/EFE was read.
+ @retval EFI_OUT_OF_RESOURCES Data or size of a FE/EFE was not read due to
+ lack of resources.
+ @retval EFI_INVALID_PARAMETER The read file flag given in ReadFileInfo is
+ invalid.
+ @retval EFI_UNSUPPORTED The FE recording flag given in FileEntryData
+ is not supported.
+ @retval other Data or size of a FE/EFE was not read.
+
+**/
EFI_STATUS
ReadFile (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -1194,9 +1395,22 @@ Error_Get_Aed:
return Status;
}
-//
-// Find a file by its filename from a given Parent file.
-//
+/**
+ Find a file by its filename from a given Parent file.
+
+ @param[in] BlockIo BlockIo interface.
+ @param[in] DiskIo DiskIo interface.
+ @param[in] Volume Volume information pointer.
+ @param[in] FileName File name string.
+ @param[in] Parent Parent directory file.
+ @param[in] Icb Long Allocation Descriptor pointer.
+ @param[out] File Found file.
+
+ @retval EFI_SUCCESS The file was found.
+ @retval EFI_INVALID_PARAMETER One or more input parameters are invalid.
+ @retval EFI_NOT_FOUND The file was not found.
+
+**/
EFI_STATUS
InternalFindFile (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
@@ -1531,13 +1745,14 @@ Error_Read_Disk_Blk:
@param[in] FilePath File's absolute path.
@param[in] Root Root directory file.
@param[in] Parent Parent directory file.
+ @param[in] Icb ICB of Parent.
@param[out] File Found file.
- @retval EFI_SUCCESS @p FilePath was found.
+ @retval EFI_SUCCESS FilePath was found.
@retval EFI_NO_MEDIA The device has no media.
@retval EFI_DEVICE_ERROR The device reported an error.
@retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
- @retval EFI_OUT_OF_RESOURCES The @p FilePath file was not found due to lack of
+ @retval EFI_OUT_OF_RESOURCES The FilePath file was not found due to lack of
resources.
**/
@@ -1658,7 +1873,7 @@ FindFile (
@param[in] Volume UDF volume information structure.
@param[in] ParentIcb ICB of the parent file.
@param[in] FileEntryData FE/EFE of the parent file.
- @param[in out] ReadDirInfo Next read directory listing structure
+ @param[in, out] ReadDirInfo Next read directory listing structure
information.
@param[out] FoundFid File Identifier Descriptor pointer.
@@ -2043,7 +2258,7 @@ CleanupFileInformation (
@param[in] File File information structure.
@param[out] Size Size of the file.
- @retval EFI_SUCCESS File size calculated and set in @p Size.
+ @retval EFI_SUCCESS File size calculated and set in Size.
@retval EFI_UNSUPPORTED Extended Allocation Descriptors not supported.
@retval EFI_NO_MEDIA The device has no media.
@retval EFI_DEVICE_ERROR The device reported an error.
@@ -2089,7 +2304,7 @@ GetFileSize (
@param[in] File File pointer.
@param[in] FileSize Size of the file.
@param[in] FileName Filename of the file.
- @param[in out] BufferSize Size of the returned file infomation.
+ @param[in, out] BufferSize Size of the returned file infomation.
@param[out] Buffer Data of the returned file information.
@retval EFI_SUCCESS File information set.
@@ -2362,9 +2577,9 @@ GetVolumeSize (
@param[in] Volume UDF volume information structure.
@param[in] File File information structure.
@param[in] FileSize Size of the file.
- @param[in out] FilePosition File position.
- @param[in out] Buffer File data.
- @param[in out] BufferSize Read size.
+ @param[in, out] FilePosition File position.
+ @param[in, out] Buffer File data.
+ @param[in, out] BufferSize Read size.
@retval EFI_SUCCESS File seeked and read.
@retval EFI_UNSUPPORTED Extended Allocation Descriptors not supported.
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index 240d420ff5..641e0ae267 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -659,7 +659,7 @@ UdfGetInfo (
/**
Set information about a file.
- @param File Protocol instance pointer.
+ @param This Protocol instance pointer.
@param InformationType Type of information in Buffer.
@param BufferSize Size of buffer.
@param Buffer The data to write.
@@ -783,13 +783,14 @@ FindFileEntry (
@param[in] FilePath File's absolute path.
@param[in] Root Root directory file.
@param[in] Parent Parent directory file.
+ @param[in] Icb ICB of Parent.
@param[out] File Found file.
- @retval EFI_SUCCESS @p FilePath was found.
+ @retval EFI_SUCCESS FilePath was found.
@retval EFI_NO_MEDIA The device has no media.
@retval EFI_DEVICE_ERROR The device reported an error.
@retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
- @retval EFI_OUT_OF_RESOURCES The @p FilePath file was not found due to lack of
+ @retval EFI_OUT_OF_RESOURCES The FilePath file was not found due to lack of
resources.
**/
@@ -813,7 +814,7 @@ FindFile (
@param[in] Volume UDF volume information structure.
@param[in] ParentIcb ICB of the parent file.
@param[in] FileEntryData FE/EFE of the parent file.
- @param[in out] ReadDirInfo Next read directory listing structure
+ @param[in, out] ReadDirInfo Next read directory listing structure
information.
@param[out] FoundFid File Identifier Descriptor pointer.
@@ -913,7 +914,7 @@ CleanupFileInformation (
@param[in] File File information structure.
@param[out] Size Size of the file.
- @retval EFI_SUCCESS File size calculated and set in @p Size.
+ @retval EFI_SUCCESS File size calculated and set in Size.
@retval EFI_UNSUPPORTED Extended Allocation Descriptors not supported.
@retval EFI_NO_MEDIA The device has no media.
@retval EFI_DEVICE_ERROR The device reported an error.
@@ -937,7 +938,7 @@ GetFileSize (
@param[in] File File pointer.
@param[in] FileSize Size of the file.
@param[in] FileName Filename of the file.
- @param[in out] BufferSize Size of the returned file infomation.
+ @param[in, out] BufferSize Size of the returned file infomation.
@param[out] Buffer Data of the returned file information.
@retval EFI_SUCCESS File information set.
@@ -991,9 +992,9 @@ GetVolumeSize (
@param[in] Volume UDF volume information structure.
@param[in] File File information structure.
@param[in] FileSize Size of the file.
- @param[in out] FilePosition File position.
- @param[in out] Buffer File data.
- @param[in out] BufferSize Read size.
+ @param[in, out] FilePosition File position.
+ @param[in, out] Buffer File data.
+ @param[in, out] BufferSize Read size.
@retval EFI_SUCCESS File seeked and read.
@retval EFI_UNSUPPORTED Extended Allocation Descriptors not supported.
@@ -1037,7 +1038,7 @@ SupportUdfFileSystem (
@param[in] FileName Filename.
- @retval @p FileName Filename mangled.
+ @retval The mangled Filename.
**/
CHAR16 *
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/7] MdeModulePkg/UdfDxe: Avoid short (single character) variable name
2017-09-15 4:57 [PATCH 0/7] MdeModulePkg/Udf: Code refinements Hao Wu
` (3 preceding siblings ...)
2017-09-15 4:57 ` [PATCH 4/7] MdeModulePkg/Udf: Refine function description comments Hao Wu
@ 2017-09-15 4:57 ` Hao Wu
2017-09-15 4:57 ` [PATCH 6/7] MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable Hao Wu
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Hao Wu @ 2017-09-15 4:57 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Paulo Alcantara, Ruiyu Ni, Star Zeng, Eric Dong,
Dandan Bi
In ResolveSymlink(), replace the following variable:
CHAR16 *C;
with:
CHAR16 *Char;
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@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 | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 8fcc508c41..90862932fd 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2045,7 +2045,7 @@ ResolveSymlink (
UDF_PATH_COMPONENT *PathComp;
UINT8 PathCompLength;
CHAR16 FileName[UDF_FILENAME_LENGTH];
- CHAR16 *C;
+ CHAR16 *Char;
UINTN Index;
UINT8 CompressionId;
UDF_FILE_INFO PreviousFile;
@@ -2122,24 +2122,24 @@ ResolveSymlink (
return EFI_VOLUME_CORRUPTED;
}
- C = FileName;
+ Char = FileName;
for (Index = 1; Index < PathCompLength; Index++) {
if (CompressionId == 16) {
- *C = *(UINT8 *)((UINT8 *)PathComp->ComponentIdentifier +
+ *Char = *(UINT8 *)((UINT8 *)PathComp->ComponentIdentifier +
Index) << 8;
Index++;
} else {
- *C = 0;
+ *Char = 0;
}
if (Index < Length) {
- *C |= (CHAR16)(*(UINT8 *)((UINT8 *)PathComp->ComponentIdentifier + Index));
+ *Char |= (CHAR16)(*(UINT8 *)((UINT8 *)PathComp->ComponentIdentifier + Index));
}
- C++;
+ Char++;
}
- *C = L'\0';
+ *Char = L'\0';
break;
}
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 6/7] MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable
2017-09-15 4:57 [PATCH 0/7] MdeModulePkg/Udf: Code refinements Hao Wu
` (4 preceding siblings ...)
2017-09-15 4:57 ` [PATCH 5/7] MdeModulePkg/UdfDxe: Avoid short (single character) variable name Hao Wu
@ 2017-09-15 4:57 ` Hao Wu
2017-09-15 4:57 ` [PATCH 7/7] MdeModulePkg/UdfDxe: Refine enum member naming style Hao Wu
2017-09-15 21:47 ` [PATCH 0/7] MdeModulePkg/Udf: Code refinements Paulo Alcantara
7 siblings, 0 replies; 11+ messages in thread
From: Hao Wu @ 2017-09-15 4:57 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Paulo Alcantara, Ruiyu Ni, Star Zeng, Eric Dong,
Dandan Bi
The local GUID variable 'UdfDevPathGuid', it has been initialized during
its declaration.
For better coding style, this commit uses a global variable instead.
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 8 ++++++--
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 8 ++++++--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index c566bfc594..609f56cef6 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -28,6 +28,11 @@ typedef struct {
} UDF_DEVICE_PATH;
//
+// Vendor-Defined Device Path GUID for UDF file system
+//
+EFI_GUID gUdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
+
+//
// Vendor-Defined Media Device Path for UDF file system
//
UDF_DEVICE_PATH gUdfDevicePath = {
@@ -260,7 +265,6 @@ PartitionInstallUdfChildHandles (
EFI_BLOCK_IO_MEDIA *Media;
EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
EFI_GUID *VendorDefinedGuid;
- EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
EFI_PARTITION_INFO_PROTOCOL PartitionInfo;
Media = BlockIo->Media;
@@ -291,7 +295,7 @@ PartitionInstallUdfChildHandles (
if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
- if (CompareGuid (VendorDefinedGuid, &UdfDevPathGuid)) {
+ if (CompareGuid (VendorDefinedGuid, &gUdfDevPathGuid)) {
return EFI_NOT_FOUND;
}
}
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 90862932fd..dfbf6b3f95 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -14,6 +14,11 @@
#include "Udf.h"
+//
+// Vendor-Defined Device Path GUID for UDF file system
+//
+EFI_GUID gUdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
+
/**
Find the anchor volume descriptor pointer.
@@ -2650,7 +2655,6 @@ SupportUdfFileSystem (
EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
EFI_DEVICE_PATH_PROTOCOL *LastDevicePathNode;
EFI_GUID *VendorDefinedGuid;
- EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
//
// Open Device Path protocol on ControllerHandle
@@ -2687,7 +2691,7 @@ SupportUdfFileSystem (
DevicePathSubType (LastDevicePathNode) == MEDIA_VENDOR_DP) {
VendorDefinedGuid = (EFI_GUID *)((UINTN)LastDevicePathNode +
OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
- if (CompareGuid (VendorDefinedGuid, &UdfDevPathGuid)) {
+ if (CompareGuid (VendorDefinedGuid, &gUdfDevPathGuid)) {
Status = EFI_SUCCESS;
}
}
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 7/7] MdeModulePkg/UdfDxe: Refine enum member naming style
2017-09-15 4:57 [PATCH 0/7] MdeModulePkg/Udf: Code refinements Hao Wu
` (5 preceding siblings ...)
2017-09-15 4:57 ` [PATCH 6/7] MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable Hao Wu
@ 2017-09-15 4:57 ` Hao Wu
2017-09-15 21:47 ` [PATCH 0/7] MdeModulePkg/Udf: Code refinements Paulo Alcantara
7 siblings, 0 replies; 11+ messages in thread
From: Hao Wu @ 2017-09-15 4:57 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Paulo Alcantara, Ruiyu Ni, Star Zeng, Eric Dong,
Dandan Bi
Similar to the naming style for variables, it's better for the name of
members in a enum type to avoid using only upper-case letters.
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@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 | 62 ++++++++++----------
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 28 ++++-----
2 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index dfbf6b3f95..5df267761f 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -710,9 +710,9 @@ GetLongAdFromAds (
// If it's either an indirect AD (Extended Alllocation Descriptor) or an
// allocated AD, then return it.
//
- ExtentFlags = GET_EXTENT_FLAGS (LONG_ADS_SEQUENCE, LongAd);
- if (ExtentFlags == EXTENT_IS_NEXT_EXTENT ||
- ExtentFlags == EXTENT_RECORDED_AND_ALLOCATED) {
+ ExtentFlags = GET_EXTENT_FLAGS (LongAdsSequence, LongAd);
+ if (ExtentFlags == ExtentIsNextExtent ||
+ ExtentFlags == ExtentRecordedAndAllocated) {
break;
}
@@ -720,7 +720,7 @@ GetLongAdFromAds (
// This AD is either not recorded but allocated, or not recorded and not
// allocated. Skip it.
//
- *Offset += AD_LENGTH (LONG_ADS_SEQUENCE);
+ *Offset += AD_LENGTH (LongAdsSequence);
}
*FoundLongAd = LongAd;
@@ -766,9 +766,9 @@ GetShortAdFromAds (
// If it's either an indirect AD (Extended Alllocation Descriptor) or an
// allocated AD, then return it.
//
- ExtentFlags = GET_EXTENT_FLAGS (SHORT_ADS_SEQUENCE, ShortAd);
- if (ExtentFlags == EXTENT_IS_NEXT_EXTENT ||
- ExtentFlags == EXTENT_RECORDED_AND_ALLOCATED) {
+ ExtentFlags = GET_EXTENT_FLAGS (ShortAdsSequence, ShortAd);
+ if (ExtentFlags == ExtentIsNextExtent ||
+ ExtentFlags == ExtentRecordedAndAllocated) {
break;
}
@@ -776,7 +776,7 @@ GetShortAdFromAds (
// This AD is either not recorded but allocated, or not recorded and not
// allocated. Skip it.
//
- *Offset += AD_LENGTH (SHORT_ADS_SEQUENCE);
+ *Offset += AD_LENGTH (ShortAdsSequence);
}
*FoundShortAd = ShortAd;
@@ -808,14 +808,14 @@ GetAllocationDescriptor (
OUT VOID **FoundAd
)
{
- if (RecordingFlags == LONG_ADS_SEQUENCE) {
+ if (RecordingFlags == LongAdsSequence) {
return GetLongAdFromAds (
Data,
Offset,
Length,
(UDF_LONG_ALLOCATION_DESCRIPTOR **)FoundAd
);
- } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
+ } else if (RecordingFlags == ShortAdsSequence) {
return GetShortAdFromAds (
Data,
Offset,
@@ -846,9 +846,9 @@ GetAllocationDescriptorLsn (
IN VOID *Ad
)
{
- if (RecordingFlags == LONG_ADS_SEQUENCE) {
+ if (RecordingFlags == LongAdsSequence) {
return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR *)Ad);
- } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
+ } else if (RecordingFlags == ShortAdsSequence) {
return GetShortAdLsn (
GetPdFromLongAd (Volume, ParentIcb),
(UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
@@ -1115,8 +1115,8 @@ ReadFile (
Data = NULL;
switch (ReadFileInfo->Flags) {
- case READ_FILE_GET_FILESIZE:
- case READ_FILE_ALLOCATE_AND_READ:
+ case ReadFileGetFileSize:
+ case ReadFileAllocateAndRead:
//
// Initialise ReadFileInfo structure for either getting file size, or
// reading file's recorded data.
@@ -1124,7 +1124,7 @@ ReadFile (
ReadFileInfo->ReadLength = 0;
ReadFileInfo->FileData = NULL;
break;
- case READ_FILE_SEEK_AND_READ:
+ case ReadFileSeekAndRead:
//
// About to seek a file and/or read its data.
//
@@ -1149,15 +1149,15 @@ ReadFile (
RecordingFlags = GET_FE_RECORDING_FLAGS (FileEntryData);
switch (RecordingFlags) {
- case INLINE_DATA:
+ case InlineData:
//
// There are no extents for this FE/EFE. All data is inline.
//
GetFileEntryData (FileEntryData, &Data, &Length);
- if (ReadFileInfo->Flags == READ_FILE_GET_FILESIZE) {
+ if (ReadFileInfo->Flags == ReadFileGetFileSize) {
ReadFileInfo->ReadLength = Length;
- } else if (ReadFileInfo->Flags == READ_FILE_ALLOCATE_AND_READ) {
+ } else if (ReadFileInfo->Flags == ReadFileAllocateAndRead) {
//
// Allocate buffer for starting read data.
//
@@ -1171,7 +1171,7 @@ ReadFile (
//
CopyMem (ReadFileInfo->FileData, Data, (UINTN) Length);
ReadFileInfo->ReadLength = Length;
- } else if (ReadFileInfo->Flags == READ_FILE_SEEK_AND_READ) {
+ } else if (ReadFileInfo->Flags == ReadFileSeekAndRead) {
//
// If FilePosition is non-zero, seek file to FilePosition, read
// FileDataSize bytes and then updates FilePosition.
@@ -1191,8 +1191,8 @@ ReadFile (
Status = EFI_SUCCESS;
break;
- case LONG_ADS_SEQUENCE:
- case SHORT_ADS_SEQUENCE:
+ case LongAdsSequence:
+ case ShortAdsSequence:
//
// This FE/EFE contains a run of Allocation Descriptors. Get data + size
// for start reading them out.
@@ -1220,7 +1220,7 @@ ReadFile (
// Check if AD is an indirect AD. If so, read Allocation Extent
// Descriptor and its extents (ADs).
//
- if (GET_EXTENT_FLAGS (RecordingFlags, Ad) == EXTENT_IS_NEXT_EXTENT) {
+ if (GET_EXTENT_FLAGS (RecordingFlags, Ad) == ExtentIsNextExtent) {
if (!DoFreeAed) {
DoFreeAed = TRUE;
} else {
@@ -1254,10 +1254,10 @@ ReadFile (
Ad);
switch (ReadFileInfo->Flags) {
- case READ_FILE_GET_FILESIZE:
+ case ReadFileGetFileSize:
ReadFileInfo->ReadLength += ExtentLength;
break;
- case READ_FILE_ALLOCATE_AND_READ:
+ case ReadFileAllocateAndRead:
//
// Increase FileData (if necessary) to read next extent.
//
@@ -1288,7 +1288,7 @@ ReadFile (
ReadFileInfo->ReadLength += ExtentLength;
break;
- case READ_FILE_SEEK_AND_READ:
+ case ReadFileSeekAndRead:
//
// Seek file first before reading in its data.
//
@@ -1364,7 +1364,7 @@ ReadFile (
}
break;
- case EXTENDED_ADS_SEQUENCE:
+ case ExtendedAdsSequence:
// FIXME: Not supported. Got no volume with it, yet.
ASSERT (FALSE);
Status = EFI_UNSUPPORTED;
@@ -1388,7 +1388,7 @@ Done:
Error_Read_Disk_Blk:
Error_Alloc_Buffer_To_Next_Ad:
- if (ReadFileInfo->Flags != READ_FILE_SEEK_AND_READ) {
+ if (ReadFileInfo->Flags != ReadFileSeekAndRead) {
FreePool (ReadFileInfo->FileData);
}
@@ -1911,7 +1911,7 @@ ReadDirectoryEntry (
// The directory's recorded data has not been read yet. So let's cache it
// into memory and the next calls won't need to read it again.
//
- ReadFileInfo.Flags = READ_FILE_ALLOCATE_AND_READ;
+ ReadFileInfo.Flags = ReadFileAllocateAndRead;
Status = ReadFile (
BlockIo,
@@ -2061,7 +2061,7 @@ ResolveSymlink (
// all its data here -- usually the data will be inline with the FE/EFE for
// lower filenames.
//
- ReadFileInfo.Flags = READ_FILE_ALLOCATE_AND_READ;
+ ReadFileInfo.Flags = ReadFileAllocateAndRead;
Status = ReadFile (
BlockIo,
@@ -2284,7 +2284,7 @@ GetFileSize (
EFI_STATUS Status;
UDF_READ_FILE_INFO ReadFileInfo;
- ReadFileInfo.Flags = READ_FILE_GET_FILESIZE;
+ ReadFileInfo.Flags = ReadFileGetFileSize;
Status = ReadFile (
BlockIo,
@@ -2610,7 +2610,7 @@ ReadFileData (
EFI_STATUS Status;
UDF_READ_FILE_INFO ReadFileInfo;
- ReadFileInfo.Flags = READ_FILE_SEEK_AND_READ;
+ ReadFileInfo.Flags = ReadFileSeekAndRead;
ReadFileInfo.FilePosition = *FilePosition;
ReadFileInfo.FileData = Buffer;
ReadFileInfo.FileDataSize = *BufferSize;
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index 641e0ae267..44c843fd4d 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -106,10 +106,10 @@
!IS_FID_PARENT_FILE (_Pointer)))
typedef enum {
- SHORT_ADS_SEQUENCE,
- LONG_ADS_SEQUENCE,
- EXTENDED_ADS_SEQUENCE,
- INLINE_DATA
+ ShortAdsSequence,
+ LongAdsSequence,
+ ExtendedAdsSequence,
+ InlineData
} UDF_FE_RECORDING_FLAGS;
#define GET_FE_RECORDING_FLAGS(_Fe) \
@@ -118,26 +118,26 @@ typedef enum {
sizeof (UDF_DESCRIPTOR_TAG)))->Flags & 0x07)
typedef enum {
- EXTENT_RECORDED_AND_ALLOCATED,
- EXTENT_NOT_RECORDED_BUT_ALLOCATED,
- EXTENT_NOT_RECORDED_NOT_ALLOCATED,
- EXTENT_IS_NEXT_EXTENT,
+ ExtentRecordedAndAllocated,
+ ExtentNotRecordedButAllocated,
+ ExtentNotRecordedNotAllocated,
+ ExtentIsNextExtent,
} UDF_EXTENT_FLAGS;
#define AD_LENGTH(_RecFlags) \
- ((_RecFlags) == SHORT_ADS_SEQUENCE ? \
+ ((_RecFlags) == ShortAdsSequence ? \
((UINT64)(sizeof (UDF_SHORT_ALLOCATION_DESCRIPTOR))) : \
((UINT64)(sizeof (UDF_LONG_ALLOCATION_DESCRIPTOR))))
#define GET_EXTENT_FLAGS(_RecFlags, _Ad) \
- ((_RecFlags) == SHORT_ADS_SEQUENCE ? \
+ ((_RecFlags) == ShortAdsSequence ? \
((UDF_EXTENT_FLAGS)((((UDF_SHORT_ALLOCATION_DESCRIPTOR *)(_Ad))->ExtentLength >> \
30) & 0x3)) : \
((UDF_EXTENT_FLAGS)((((UDF_LONG_ALLOCATION_DESCRIPTOR *)(_Ad))->ExtentLength >> \
30) & 0x3)))
#define GET_EXTENT_LENGTH(_RecFlags, _Ad) \
- ((_RecFlags) == SHORT_ADS_SEQUENCE ? \
+ ((_RecFlags) == ShortAdsSequence ? \
((UINT32)((((UDF_SHORT_ALLOCATION_DESCRIPTOR *)(_Ad))->ExtentLength & \
~0xC0000000UL))) : \
((UINT32)((((UDF_LONG_ALLOCATION_DESCRIPTOR *)(_Ad))->ExtentLength & \
@@ -169,9 +169,9 @@ typedef struct {
#pragma pack()
typedef enum {
- READ_FILE_GET_FILESIZE,
- READ_FILE_ALLOCATE_AND_READ,
- READ_FILE_SEEK_AND_READ,
+ ReadFileGetFileSize,
+ ReadFileAllocateAndRead,
+ ReadFileSeekAndRead,
} UDF_READ_FILE_FLAGS;
typedef struct {
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 0/7] MdeModulePkg/Udf: Code refinements
2017-09-15 4:57 [PATCH 0/7] MdeModulePkg/Udf: Code refinements Hao Wu
` (6 preceding siblings ...)
2017-09-15 4:57 ` [PATCH 7/7] MdeModulePkg/UdfDxe: Refine enum member naming style Hao Wu
@ 2017-09-15 21:47 ` Paulo Alcantara
2017-09-19 3:30 ` Zeng, Star
7 siblings, 1 reply; 11+ messages in thread
From: Paulo Alcantara @ 2017-09-15 21:47 UTC (permalink / raw)
To: Hao Wu, edk2-devel; +Cc: Ruiyu Ni, Star Zeng, Eric Dong, Dandan Bi
Hi Hao,
On 15/09/2017 01:57, Hao Wu wrote:
> The series introduces the following code refinements for UdfDxe &
> PartitionDxe:
>
> a. Add checks to ensure no possible NULL pointer dereference
> b. Reslove operands of different size in bitwise operations
> c. Use compare operator for non-boolean comparisons
> d. Refine function description comments
> e. Refine local variable initialization
> f. Refine enum members naming style
>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
>
> Hao Wu (7):
> MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref
> MdeModulePkg/UdfDxe: Fix operands of different size in bitwise OP
> MdeModulePkg/UdfDxe: Use compare operator for non-boolean comparisons
> MdeModulePkg/Udf: Refine function description comments
> MdeModulePkg/UdfDxe: Avoid short (single character) variable name
> MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable
> MdeModulePkg/UdfDxe: Refine enum member naming style
>
> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 20 +-
> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 5 +-
> MdeModulePkg/Universal/Disk/UdfDxe/FileName.c | 27 +-
> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 483 +++++++++++++++-----
> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 49 +-
> 5 files changed, 432 insertions(+), 152 deletions(-)
Looks good to me. Also tested it with OVMF X64. Thanks!
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Paulo
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/7] MdeModulePkg/Udf: Code refinements
2017-09-15 21:47 ` [PATCH 0/7] MdeModulePkg/Udf: Code refinements Paulo Alcantara
@ 2017-09-19 3:30 ` Zeng, Star
2017-09-19 3:36 ` Wu, Hao A
0 siblings, 1 reply; 11+ messages in thread
From: Zeng, Star @ 2017-09-19 3:30 UTC (permalink / raw)
To: Paulo Alcantara, Wu, Hao A, edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Dong, Eric, Bi, Dandan, Zeng, Star
Thanks for keeping improving the UDF code.
Reviewed-by: Star Zeng <star.zeng@intel.com>
Hao, you may push this patch series first, after that, could you help kindly check whether are there similar issues with the new patch series at https://lists.01.org/pipermail/edk2-devel/2017-September/014791.html?
Thanks,
Star
-----Original Message-----
From: Paulo Alcantara [mailto:pcacjr@zytor.com]
Sent: Saturday, September 16, 2017 5:47 AM
To: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Bi, Dandan <dandan.bi@intel.com>
Subject: Re: [PATCH 0/7] MdeModulePkg/Udf: Code refinements
Hi Hao,
On 15/09/2017 01:57, Hao Wu wrote:
> The series introduces the following code refinements for UdfDxe &
> PartitionDxe:
>
> a. Add checks to ensure no possible NULL pointer dereference b.
> Reslove operands of different size in bitwise operations c. Use
> compare operator for non-boolean comparisons d. Refine function
> description comments e. Refine local variable initialization f. Refine
> enum members naming style
>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
>
> Hao Wu (7):
> MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref
> MdeModulePkg/UdfDxe: Fix operands of different size in bitwise OP
> MdeModulePkg/UdfDxe: Use compare operator for non-boolean comparisons
> MdeModulePkg/Udf: Refine function description comments
> MdeModulePkg/UdfDxe: Avoid short (single character) variable name
> MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable
> MdeModulePkg/UdfDxe: Refine enum member naming style
>
> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 20 +-
> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 5 +-
> MdeModulePkg/Universal/Disk/UdfDxe/FileName.c | 27 +-
> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 483 +++++++++++++++-----
> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 49 +-
> 5 files changed, 432 insertions(+), 152 deletions(-)
Looks good to me. Also tested it with OVMF X64. Thanks!
Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
Paulo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/7] MdeModulePkg/Udf: Code refinements
2017-09-19 3:30 ` Zeng, Star
@ 2017-09-19 3:36 ` Wu, Hao A
0 siblings, 0 replies; 11+ messages in thread
From: Wu, Hao A @ 2017-09-19 3:36 UTC (permalink / raw)
To: Zeng, Star, Paulo Alcantara, edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Dong, Eric, Bi, Dandan
Sure, I will keep an eye on the changes in UDF-related code.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, September 19, 2017 11:31 AM
> To: Paulo Alcantara; Wu, Hao A; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Dong, Eric; Bi, Dandan; Zeng, Star
> Subject: RE: [PATCH 0/7] MdeModulePkg/Udf: Code refinements
>
> Thanks for keeping improving the UDF code.
> Reviewed-by: Star Zeng <star.zeng@intel.com>
>
> Hao, you may push this patch series first, after that, could you help kindly check
> whether are there similar issues with the new patch series at
> https://lists.01.org/pipermail/edk2-devel/2017-September/014791.html?
>
>
> Thanks,
> Star
> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Saturday, September 16, 2017 5:47 AM
> To: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong,
> Eric <eric.dong@intel.com>; Bi, Dandan <dandan.bi@intel.com>
> Subject: Re: [PATCH 0/7] MdeModulePkg/Udf: Code refinements
>
> Hi Hao,
>
> On 15/09/2017 01:57, Hao Wu wrote:
> > The series introduces the following code refinements for UdfDxe &
> > PartitionDxe:
> >
> > a. Add checks to ensure no possible NULL pointer dereference b.
> > Reslove operands of different size in bitwise operations c. Use
> > compare operator for non-boolean comparisons d. Refine function
> > description comments e. Refine local variable initialization f. Refine
> > enum members naming style
> >
> > Cc: Paulo Alcantara <pcacjr@zytor.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> >
> > Hao Wu (7):
> > MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref
> > MdeModulePkg/UdfDxe: Fix operands of different size in bitwise OP
> > MdeModulePkg/UdfDxe: Use compare operator for non-boolean
> comparisons
> > MdeModulePkg/Udf: Refine function description comments
> > MdeModulePkg/UdfDxe: Avoid short (single character) variable name
> > MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable
> > MdeModulePkg/UdfDxe: Refine enum member naming style
> >
> > MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 20 +-
> > MdeModulePkg/Universal/Disk/UdfDxe/File.c | 5 +-
> > MdeModulePkg/Universal/Disk/UdfDxe/FileName.c | 27 +-
> > MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 483
> +++++++++++++++-----
> > MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 49 +-
> > 5 files changed, 432 insertions(+), 152 deletions(-)
> Looks good to me. Also tested it with OVMF X64. Thanks!
>
> Reviewed-by: Paulo Alcantara <pcacjr@zytor.com>
>
> Paulo
^ permalink raw reply [flat|nested] 11+ messages in thread