public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/7] MdeModulePkg/Udf: Code refinements
@ 2017-09-15  4:57 Hao Wu
  2017-09-15  4:57 ` [PATCH 1/7] MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref Hao Wu
                   ` (7 more replies)
  0 siblings, 8 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 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(-)

-- 
2.12.0.windows.1



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

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

end of thread, other threads:[~2017-09-19  3:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [PATCH 3/7] MdeModulePkg/UdfDxe: Use compare operator for non-boolean comparisons Hao Wu
2017-09-15  4:57 ` [PATCH 4/7] MdeModulePkg/Udf: Refine function description comments Hao Wu
2017-09-15  4:57 ` [PATCH 5/7] MdeModulePkg/UdfDxe: Avoid short (single character) variable name Hao Wu
2017-09-15  4:57 ` [PATCH 6/7] MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable 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
2017-09-19  3:30   ` Zeng, Star
2017-09-19  3:36     ` Wu, Hao A

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