[PATCH 0/6] Refine code logics to prevent possible mis-reports by static code checkers

[PATCH 0/6] Refine code logics to prevent possible mis-reports by static code checkers

[Hao Wu]

The series refines the loop logic (e.g. for, while) of some functions to
be more straightforward. This will help to prevent some possible
mis-reports by static code checkers

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>

Hao Wu (6):
  MdePkg/BaseLib: Refine (Ascii)StrnLenS functions logic
  MdePkg/BaseLib: Add an additional check within (Ascii)StrnCmp
  MdePkg/MemoryLib: Refine InternalMemSetMem16|32|64 functions logic
  MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic
  MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic
  NetworkPkg: Refine UintnToAscDecWithFormat functions logic

 MdeModulePkg/Library/DxeNetLib/NetBuffer.c             | 16 ++++++++--------
 .../Universal/Network/UefiPxeBcDxe/PxeBcSupport.c      |  5 ++---
 MdePkg/Library/BaseLib/SafeString.c                    | 16 ++++++++++++----
 MdePkg/Library/BaseLib/String.c                        |  4 +++-
 MdePkg/Library/BaseMemoryLib/MemLibGeneric.c           | 18 +++++++++---------
 MdePkg/Library/PeiMemoryLib/MemLibGeneric.c            | 18 +++++++++---------
 MdePkg/Library/UefiMemoryLib/MemLibGeneric.c           | 18 +++++++++---------
 NetworkPkg/HttpBootDxe/HttpBootSupport.c               |  5 ++---
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c                 |  5 ++---
 9 files changed, 56 insertions(+), 49 deletions(-)

-- 
1.9.5.msysgit.0

[PATCH 1/6] MdePkg/BaseLib: Refine (Ascii)StrnLenS functions logic

[Hao Wu]

This commit refines the logic for AsciiStrnLenS and StrnLenS. It makes the
logic more straightforward to prevent possible mis-reports by static code
checkers.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdePkg/Library/BaseLib/SafeString.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c
index ede2f4c..3247d28 100644
--- a/MdePkg/Library/BaseLib/SafeString.c
+++ b/MdePkg/Library/BaseLib/SafeString.c
@@ -143,8 +143,12 @@ StrnLenS (
   // String then StrnLenS returns MaxSize. At most the first MaxSize characters of String shall
   // be accessed by StrnLenS.
   //
-  for (Length = 0; (Length < MaxSize) && (*String != 0); String++, Length++) {
-    ;
+  Length = 0;
+  while (String[Length] != 0) {
+    if (Length >= MaxSize) {
+      break;
+    }
+    Length++;
   }
   return Length;
 }
@@ -571,8 +575,12 @@ AsciiStrnLenS (
   // String then AsciiStrnLenS returns MaxSize. At most the first MaxSize characters of String shall
   // be accessed by AsciiStrnLenS.
   //
-  for (Length = 0; (Length < MaxSize) && (*String != 0); String++, Length++) {
-    ;
+  Length = 0;
+  while (String[Length] != 0) {
+    if (Length >= MaxSize) {
+      break;
+    }
+    Length++;
   }
   return Length;
 }
-- 
1.9.5.msysgit.0

[PATCH 2/6] MdePkg/BaseLib: Add an additional check within (Ascii)StrnCmp

[Hao Wu]

This commit adds an addtional check in AsciiStrnCmp and StrnCmp. It
explicitly checks the end of the sting pointed by 'SecondString' to make
the code logic easier for reading and to prevent possible mis-reports by
static code checkers.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdePkg/Library/BaseLib/String.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
index 25962f8..fa96d1c 100644
--- a/MdePkg/Library/BaseLib/String.c
+++ b/MdePkg/Library/BaseLib/String.c
@@ -1,7 +1,7 @@
 /** @file
   Unicode and ASCII string primitives.
 
-  Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -315,6 +315,7 @@ StrnCmp (
   }
 
   while ((*FirstString != L'\0') &&
+         (*SecondString != L'\0') &&
          (*FirstString == *SecondString) &&
          (Length > 1)) {
     FirstString++;
@@ -1474,6 +1475,7 @@ AsciiStrnCmp (
   }
 
   while ((*FirstString != '\0') &&
+         (*SecondString != '\0') &&
          (*FirstString == *SecondString) &&
          (Length > 1)) {
     FirstString++;
-- 
1.9.5.msysgit.0

[PATCH 3/6] MdePkg/MemoryLib: Refine InternalMemSetMem16|32|64 functions logic

[Hao Wu]

This commit refines the logic for InternalMemSetMem16|32|64 functions. It
avoids using the decrement operator '--' for array index to prevent
possible mis-reports by static code checkers.

Please note that those modified functions are only consumed within
MemoryLib by APIs SetMem16|32|64, and those APIs will handle the case when
the input number of bytes to set is 0. Hence, the behavior of APIs
SetMem16|32|64 is not changed.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdePkg/Library/BaseMemoryLib/MemLibGeneric.c | 18 +++++++++---------
 MdePkg/Library/PeiMemoryLib/MemLibGeneric.c  | 18 +++++++++---------
 MdePkg/Library/UefiMemoryLib/MemLibGeneric.c | 18 +++++++++---------
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/MdePkg/Library/BaseMemoryLib/MemLibGeneric.c b/MdePkg/Library/BaseMemoryLib/MemLibGeneric.c
index b058be8..cf40ace 100644
--- a/MdePkg/Library/BaseMemoryLib/MemLibGeneric.c
+++ b/MdePkg/Library/BaseMemoryLib/MemLibGeneric.c
@@ -37,9 +37,9 @@ InternalMemSetMem16 (
   IN      UINT16                    Value
   )
 {
-  do {
-    ((UINT16*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT16*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
@@ -61,9 +61,9 @@ InternalMemSetMem32 (
   IN      UINT32                    Value
   )
 {
-  do {
-    ((UINT32*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT32*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
@@ -85,9 +85,9 @@ InternalMemSetMem64 (
   IN      UINT64                    Value
   )
 {
-  do {
-    ((UINT64*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT64*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
diff --git a/MdePkg/Library/PeiMemoryLib/MemLibGeneric.c b/MdePkg/Library/PeiMemoryLib/MemLibGeneric.c
index 490b244..ed18b57 100644
--- a/MdePkg/Library/PeiMemoryLib/MemLibGeneric.c
+++ b/MdePkg/Library/PeiMemoryLib/MemLibGeneric.c
@@ -37,9 +37,9 @@ InternalMemSetMem16 (
   IN      UINT16                    Value
   )
 {
-  do {
-    ((UINT16*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT16*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
@@ -61,9 +61,9 @@ InternalMemSetMem32 (
   IN      UINT32                    Value
   )
 {
-  do {
-    ((UINT32*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT32*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
@@ -85,9 +85,9 @@ InternalMemSetMem64 (
   IN      UINT64                    Value
   )
 {
-  do {
-    ((UINT64*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT64*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
diff --git a/MdePkg/Library/UefiMemoryLib/MemLibGeneric.c b/MdePkg/Library/UefiMemoryLib/MemLibGeneric.c
index da02b6c..f1efdbb 100644
--- a/MdePkg/Library/UefiMemoryLib/MemLibGeneric.c
+++ b/MdePkg/Library/UefiMemoryLib/MemLibGeneric.c
@@ -37,9 +37,9 @@ InternalMemSetMem16 (
   IN      UINT16                    Value
   )
 {
-  do {
-    ((UINT16*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT16*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
@@ -61,9 +61,9 @@ InternalMemSetMem32 (
   IN      UINT32                    Value
   )
 {
-  do {
-    ((UINT32*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT32*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
@@ -85,9 +85,9 @@ InternalMemSetMem64 (
   IN      UINT64                    Value
   )
 {
-  do {
-    ((UINT64*)Buffer)[--Length] = Value;
-  } while (Length != 0);
+  for (; Length != 0; Length--) {
+    ((UINT64*)Buffer)[Length - 1] = Value;
+  }
   return Buffer;
 }
 
-- 
1.9.5.msysgit.0

[PATCH 4/6] MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic

[Hao Wu]

This commit rewrites the logic for NetblockChecksum. It processes the
checksum of the left-over byte first to prevent possible mis-reports by
static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Library/DxeNetLib/NetBuffer.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
index bbbdbc0..95cb717 100644
--- a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
+++ b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
@@ -1,7 +1,7 @@
 /** @file
   Network library functions providing net buffer operation support.
 
-Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -1661,6 +1661,13 @@ NetblockChecksum (
 
   Sum = 0;
 
+  //
+  // Add left-over byte, if any
+  //
+  if (Len % 2 != 0) {
+    Sum += *(Bulk + Len - 1);
+  }
+
   while (Len > 1) {
     Sum += *(UINT16 *) Bulk;
     Bulk += 2;
@@ -1668,13 +1675,6 @@ NetblockChecksum (
   }
 
   //
-  // Add left-over byte, if any
-  //
-  if (Len > 0) {
-    Sum += *(UINT8 *) Bulk;
-  }
-
-  //
   // Fold 32-bit sum to 16 bits
   //
   while ((Sum >> 16) != 0) {
-- 
1.9.5.msysgit.0

[PATCH 5/6] MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic

[Hao Wu]

This commit refines the logic for the CvtNum function. It avoids using the
decrement operator '--' for array index to prevent possible mis-reports by
static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
index 0865ddd..0779056 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
@@ -132,11 +132,10 @@ CvtNum (
 {
   UINTN Remainder;
 
-  while (Length > 0) {
+  for (; Length > 0; Length--) {
     Remainder = Number % 10;
     Number /= 10;
-    Length--;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
-- 
1.9.5.msysgit.0

[PATCH 6/6] NetworkPkg: Refine UintnToAscDecWithFormat functions logic

[Hao Wu]

This commit refines the logic for HttpBootUintnToAscDecWithFormat and
PxeBcUintnToAscDecWithFormat. It avoids using the decrement operator '--'
for array index to prevent possible mis-reports by static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 NetworkPkg/HttpBootDxe/HttpBootSupport.c | 5 ++---
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c   | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
index 9410bf9..bdb29ae 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
@@ -86,11 +86,10 @@ HttpBootUintnToAscDecWithFormat (
 {
   UINTN                          Remainder;
 
-  while (Length > 0) {
-    Length--;
+  for (; Length > 0; Length--) {
     Remainder      = Number % 10;
     Number        /= 10;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
index 00c652d..568360d 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
@@ -1383,11 +1383,10 @@ PxeBcUintnToAscDecWithFormat (
 {
   UINTN                          Remainder;
 
-  while (Length > 0) {
-    Length--;
+  for (; Length > 0; Length--) {
     Remainder      = Number % 10;
     Number        /= 10;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
-- 
1.9.5.msysgit.0

Re: [PATCH 4/6] MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic

[Fu, Siyuan]

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>


-----Original Message-----
From: Wu, Hao A 
Sent: 2016年12月14日 19:27
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 4/6] MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic

This commit rewrites the logic for NetblockChecksum. It processes the checksum of the left-over byte first to prevent possible mis-reports by static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Library/DxeNetLib/NetBuffer.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
index bbbdbc0..95cb717 100644
--- a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
+++ b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
@@ -1,7 +1,7 @@
 /** @file
   Network library functions providing net buffer operation support.
 
-Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at @@ -1661,6 +1661,13 @@ NetblockChecksum (
 
   Sum = 0;
 
+  //
+  // Add left-over byte, if any
+  //
+  if (Len % 2 != 0) {
+    Sum += *(Bulk + Len - 1);
+  }
+
   while (Len > 1) {
     Sum += *(UINT16 *) Bulk;
     Bulk += 2;
@@ -1668,13 +1675,6 @@ NetblockChecksum (
   }
 
   //
-  // Add left-over byte, if any
-  //
-  if (Len > 0) {
-    Sum += *(UINT8 *) Bulk;
-  }
-
-  //
   // Fold 32-bit sum to 16 bits
   //
   while ((Sum >> 16) != 0) {
--
1.9.5.msysgit.0

Re: [PATCH 5/6] MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic

[Fu, Siyuan]

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>


-----Original Message-----
From: Wu, Hao A 
Sent: 2016年12月14日 19:27
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 5/6] MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic

This commit refines the logic for the CvtNum function. It avoids using the decrement operator '--' for array index to prevent possible mis-reports by static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
index 0865ddd..0779056 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
@@ -132,11 +132,10 @@ CvtNum (
 {
   UINTN Remainder;
 
-  while (Length > 0) {
+  for (; Length > 0; Length--) {
     Remainder = Number % 10;
     Number /= 10;
-    Length--;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
--
1.9.5.msysgit.0

Re: [PATCH 6/6] NetworkPkg: Refine UintnToAscDecWithFormat functions logic

[Fu, Siyuan]

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>


-----Original Message-----
From: Wu, Hao A 
Sent: 2016年12月14日 19:27
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 6/6] NetworkPkg: Refine UintnToAscDecWithFormat functions logic

This commit refines the logic for HttpBootUintnToAscDecWithFormat and PxeBcUintnToAscDecWithFormat. It avoids using the decrement operator '--'
for array index to prevent possible mis-reports by static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 NetworkPkg/HttpBootDxe/HttpBootSupport.c | 5 ++---
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c   | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
index 9410bf9..bdb29ae 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
@@ -86,11 +86,10 @@ HttpBootUintnToAscDecWithFormat (  {
   UINTN                          Remainder;
 
-  while (Length > 0) {
-    Length--;
+  for (; Length > 0; Length--) {
     Remainder      = Number % 10;
     Number        /= 10;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
index 00c652d..568360d 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
@@ -1383,11 +1383,10 @@ PxeBcUintnToAscDecWithFormat (  {
   UINTN                          Remainder;
 
-  while (Length > 0) {
-    Length--;
+  for (; Length > 0; Length--) {
     Remainder      = Number % 10;
     Number        /= 10;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
--
1.9.5.msysgit.0

Re: [PATCH 6/6] NetworkPkg: Refine UintnToAscDecWithFormat functions logic

[Ye, Ting]

Reviewed-by: Ye Ting <ting.ye@intel.com> 

-----Original Message-----
From: Wu, Hao A 
Sent: Wednesday, December 14, 2016 7:27 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 6/6] NetworkPkg: Refine UintnToAscDecWithFormat functions logic

This commit refines the logic for HttpBootUintnToAscDecWithFormat and PxeBcUintnToAscDecWithFormat. It avoids using the decrement operator '--'
for array index to prevent possible mis-reports by static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 NetworkPkg/HttpBootDxe/HttpBootSupport.c | 5 ++---
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c   | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
index 9410bf9..bdb29ae 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
@@ -86,11 +86,10 @@ HttpBootUintnToAscDecWithFormat (  {
   UINTN                          Remainder;
 
-  while (Length > 0) {
-    Length--;
+  for (; Length > 0; Length--) {
     Remainder      = Number % 10;
     Number        /= 10;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
index 00c652d..568360d 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
@@ -1383,11 +1383,10 @@ PxeBcUintnToAscDecWithFormat (  {
   UINTN                          Remainder;
 
-  while (Length > 0) {
-    Length--;
+  for (; Length > 0; Length--) {
     Remainder      = Number % 10;
     Number        /= 10;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
--
1.9.5.msysgit.0

Re: [PATCH 5/6] MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic

[Ye, Ting]

Reviewed-by: Ye Ting <ting.ye@intel.com>


-----Original Message-----
From: Wu, Hao A 
Sent: Wednesday, December 14, 2016 7:27 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 5/6] MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic

This commit refines the logic for the CvtNum function. It avoids using the decrement operator '--' for array index to prevent possible mis-reports by static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
index 0865ddd..0779056 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
@@ -132,11 +132,10 @@ CvtNum (
 {
   UINTN Remainder;
 
-  while (Length > 0) {
+  for (; Length > 0; Length--) {
     Remainder = Number % 10;
     Number /= 10;
-    Length--;
-    Buffer[Length] = (UINT8) ('0' + Remainder);
+    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
   }
 }
 
--
1.9.5.msysgit.0

Re: [PATCH 4/6] MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic

[Ye, Ting]

Reviewed-by: Ye Ting <ting.ye@intel.com>


-----Original Message-----
From: Wu, Hao A 
Sent: Wednesday, December 14, 2016 7:27 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [PATCH 4/6] MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic

This commit rewrites the logic for NetblockChecksum. It processes the checksum of the left-over byte first to prevent possible mis-reports by static code checkers.

Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Library/DxeNetLib/NetBuffer.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
index bbbdbc0..95cb717 100644
--- a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
+++ b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
@@ -1,7 +1,7 @@
 /** @file
   Network library functions providing net buffer operation support.
 
-Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at @@ -1661,6 +1661,13 @@ NetblockChecksum (
 
   Sum = 0;
 
+  //
+  // Add left-over byte, if any
+  //
+  if (Len % 2 != 0) {
+    Sum += *(Bulk + Len - 1);
+  }
+
   while (Len > 1) {
     Sum += *(UINT16 *) Bulk;
     Bulk += 2;
@@ -1668,13 +1675,6 @@ NetblockChecksum (
   }
 
   //
-  // Add left-over byte, if any
-  //
-  if (Len > 0) {
-    Sum += *(UINT8 *) Bulk;
-  }
-
-  //
   // Fold 32-bit sum to 16 bits
   //
   while ((Sum >> 16) != 0) {
--
1.9.5.msysgit.0

Re: [PATCH 4/6] MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic

[Wu, Jiaxin]

Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>


> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, December 14, 2016 7:27 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye,
> Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH 4/6] MdeModulePkg/DxeNetLib: Rewrite
> NetblockChecksum function logic
> 
> This commit rewrites the logic for NetblockChecksum. It processes the
> checksum of the left-over byte first to prevent possible mis-reports by static
> code checkers.
> 
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Library/DxeNetLib/NetBuffer.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
> b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
> index bbbdbc0..95cb717 100644
> --- a/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
> +++ b/MdeModulePkg/Library/DxeNetLib/NetBuffer.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Network library functions providing net buffer operation support.
> 
> -Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials  are licensed and made
> available under the terms and conditions of the BSD License  which
> accompanies this distribution.  The full text of the license may be found at
> @@ -1661,6 +1661,13 @@ NetblockChecksum (
> 
>    Sum = 0;
> 
> +  //
> +  // Add left-over byte, if any
> +  //
> +  if (Len % 2 != 0) {
> +    Sum += *(Bulk + Len - 1);
> +  }
> +
>    while (Len > 1) {
>      Sum += *(UINT16 *) Bulk;
>      Bulk += 2;
> @@ -1668,13 +1675,6 @@ NetblockChecksum (
>    }
> 
>    //
> -  // Add left-over byte, if any
> -  //
> -  if (Len > 0) {
> -    Sum += *(UINT8 *) Bulk;
> -  }
> -
> -  //
>    // Fold 32-bit sum to 16 bits
>    //
>    while ((Sum >> 16) != 0) {
> --
> 1.9.5.msysgit.0

Re: [PATCH 5/6] MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic

[Wu, Jiaxin]

Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>


> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, December 14, 2016 7:27 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye,
> Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH 5/6] MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum
> function logic
> 
> This commit refines the logic for the CvtNum function. It avoids using the
> decrement operator '--' for array index to prevent possible mis-reports by
> static code checkers.
> 
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c | 5 ++--
> -
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
> b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
> index 0865ddd..0779056 100644
> --- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
> +++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c
> @@ -132,11 +132,10 @@ CvtNum (
>  {
>    UINTN Remainder;
> 
> -  while (Length > 0) {
> +  for (; Length > 0; Length--) {
>      Remainder = Number % 10;
>      Number /= 10;
> -    Length--;
> -    Buffer[Length] = (UINT8) ('0' + Remainder);
> +    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
>    }
>  }
> 
> --
> 1.9.5.msysgit.0

Re: [PATCH 6/6] NetworkPkg: Refine UintnToAscDecWithFormat functions logic

[Wu, Jiaxin]

Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>


> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, December 14, 2016 7:27 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye,
> Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH 6/6] NetworkPkg: Refine UintnToAscDecWithFormat
> functions logic
> 
> This commit refines the logic for HttpBootUintnToAscDecWithFormat and
> PxeBcUintnToAscDecWithFormat. It avoids using the decrement operator '--'
> for array index to prevent possible mis-reports by static code checkers.
> 
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c | 5 ++---
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c   | 5 ++---
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> index 9410bf9..bdb29ae 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> @@ -86,11 +86,10 @@ HttpBootUintnToAscDecWithFormat (  {
>    UINTN                          Remainder;
> 
> -  while (Length > 0) {
> -    Length--;
> +  for (; Length > 0; Length--) {
>      Remainder      = Number % 10;
>      Number        /= 10;
> -    Buffer[Length] = (UINT8) ('0' + Remainder);
> +    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
>    }
>  }
> 
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
> index 00c652d..568360d 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
> @@ -1383,11 +1383,10 @@ PxeBcUintnToAscDecWithFormat (  {
>    UINTN                          Remainder;
> 
> -  while (Length > 0) {
> -    Length--;
> +  for (; Length > 0; Length--) {
>      Remainder      = Number % 10;
>      Number        /= 10;
> -    Buffer[Length] = (UINT8) ('0' + Remainder);
> +    Buffer[Length - 1] = (UINT8) ('0' + Remainder);
>    }
>  }
> 
> --
> 1.9.5.msysgit.0

Re: [PATCH 1/6] MdePkg/BaseLib: Refine (Ascii)StrnLenS functions logic

[Kinney, Michael D]

Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Hao Wu
> Sent: Wednesday, December 14, 2016 3:27 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [PATCH 1/6] MdePkg/BaseLib: Refine (Ascii)StrnLenS functions logic
> 
> This commit refines the logic for AsciiStrnLenS and StrnLenS. It makes the
> logic more straightforward to prevent possible mis-reports by static code
> checkers.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdePkg/Library/BaseLib/SafeString.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/SafeString.c
> b/MdePkg/Library/BaseLib/SafeString.c
> index ede2f4c..3247d28 100644
> --- a/MdePkg/Library/BaseLib/SafeString.c
> +++ b/MdePkg/Library/BaseLib/SafeString.c
> @@ -143,8 +143,12 @@ StrnLenS (
>    // String then StrnLenS returns MaxSize. At most the first MaxSize characters of
> String shall
>    // be accessed by StrnLenS.
>    //
> -  for (Length = 0; (Length < MaxSize) && (*String != 0); String++, Length++) {
> -    ;
> +  Length = 0;
> +  while (String[Length] != 0) {
> +    if (Length >= MaxSize) {
> +      break;
> +    }
> +    Length++;
>    }
>    return Length;
>  }
> @@ -571,8 +575,12 @@ AsciiStrnLenS (
>    // String then AsciiStrnLenS returns MaxSize. At most the first MaxSize characters
> of String shall
>    // be accessed by AsciiStrnLenS.
>    //
> -  for (Length = 0; (Length < MaxSize) && (*String != 0); String++, Length++) {
> -    ;
> +  Length = 0;
> +  while (String[Length] != 0) {
> +    if (Length >= MaxSize) {
> +      break;
> +    }
> +    Length++;
>    }
>    return Length;
>  }
> --
> 1.9.5.msysgit.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 2/6] MdePkg/BaseLib: Add an additional check within (Ascii)StrnCmp

[Kinney, Michael D]

Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>

> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, December 14, 2016 3:27 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH 2/6] MdePkg/BaseLib: Add an additional check within (Ascii)StrnCmp
> 
> This commit adds an addtional check in AsciiStrnCmp and StrnCmp. It
> explicitly checks the end of the sting pointed by 'SecondString' to make
> the code logic easier for reading and to prevent possible mis-reports by
> static code checkers.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdePkg/Library/BaseLib/String.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
> index 25962f8..fa96d1c 100644
> --- a/MdePkg/Library/BaseLib/String.c
> +++ b/MdePkg/Library/BaseLib/String.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Unicode and ASCII string primitives.
> 
> -  Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
>    which accompanies this distribution.  The full text of the license may be found at
> @@ -315,6 +315,7 @@ StrnCmp (
>    }
> 
>    while ((*FirstString != L'\0') &&
> +         (*SecondString != L'\0') &&
>           (*FirstString == *SecondString) &&
>           (Length > 1)) {
>      FirstString++;
> @@ -1474,6 +1475,7 @@ AsciiStrnCmp (
>    }
> 
>    while ((*FirstString != '\0') &&
> +         (*SecondString != '\0') &&
>           (*FirstString == *SecondString) &&
>           (Length > 1)) {
>      FirstString++;
> --
> 1.9.5.msysgit.0

Re: [PATCH 3/6] MdePkg/MemoryLib: Refine InternalMemSetMem16|32|64 functions logic

[Kinney, Michael D]

Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>

> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, December 14, 2016 3:27 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH 3/6] MdePkg/MemoryLib: Refine InternalMemSetMem16|32|64 functions
> logic
> 
> This commit refines the logic for InternalMemSetMem16|32|64 functions. It
> avoids using the decrement operator '--' for array index to prevent
> possible mis-reports by static code checkers.
> 
> Please note that those modified functions are only consumed within
> MemoryLib by APIs SetMem16|32|64, and those APIs will handle the case when
> the input number of bytes to set is 0. Hence, the behavior of APIs
> SetMem16|32|64 is not changed.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdePkg/Library/BaseMemoryLib/MemLibGeneric.c | 18 +++++++++---------
>  MdePkg/Library/PeiMemoryLib/MemLibGeneric.c  | 18 +++++++++---------
>  MdePkg/Library/UefiMemoryLib/MemLibGeneric.c | 18 +++++++++---------
>  3 files changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseMemoryLib/MemLibGeneric.c
> b/MdePkg/Library/BaseMemoryLib/MemLibGeneric.c
> index b058be8..cf40ace 100644
> --- a/MdePkg/Library/BaseMemoryLib/MemLibGeneric.c
> +++ b/MdePkg/Library/BaseMemoryLib/MemLibGeneric.c
> @@ -37,9 +37,9 @@ InternalMemSetMem16 (
>    IN      UINT16                    Value
>    )
>  {
> -  do {
> -    ((UINT16*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT16*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> @@ -61,9 +61,9 @@ InternalMemSetMem32 (
>    IN      UINT32                    Value
>    )
>  {
> -  do {
> -    ((UINT32*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT32*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> @@ -85,9 +85,9 @@ InternalMemSetMem64 (
>    IN      UINT64                    Value
>    )
>  {
> -  do {
> -    ((UINT64*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT64*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> diff --git a/MdePkg/Library/PeiMemoryLib/MemLibGeneric.c
> b/MdePkg/Library/PeiMemoryLib/MemLibGeneric.c
> index 490b244..ed18b57 100644
> --- a/MdePkg/Library/PeiMemoryLib/MemLibGeneric.c
> +++ b/MdePkg/Library/PeiMemoryLib/MemLibGeneric.c
> @@ -37,9 +37,9 @@ InternalMemSetMem16 (
>    IN      UINT16                    Value
>    )
>  {
> -  do {
> -    ((UINT16*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT16*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> @@ -61,9 +61,9 @@ InternalMemSetMem32 (
>    IN      UINT32                    Value
>    )
>  {
> -  do {
> -    ((UINT32*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT32*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> @@ -85,9 +85,9 @@ InternalMemSetMem64 (
>    IN      UINT64                    Value
>    )
>  {
> -  do {
> -    ((UINT64*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT64*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> diff --git a/MdePkg/Library/UefiMemoryLib/MemLibGeneric.c
> b/MdePkg/Library/UefiMemoryLib/MemLibGeneric.c
> index da02b6c..f1efdbb 100644
> --- a/MdePkg/Library/UefiMemoryLib/MemLibGeneric.c
> +++ b/MdePkg/Library/UefiMemoryLib/MemLibGeneric.c
> @@ -37,9 +37,9 @@ InternalMemSetMem16 (
>    IN      UINT16                    Value
>    )
>  {
> -  do {
> -    ((UINT16*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT16*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> @@ -61,9 +61,9 @@ InternalMemSetMem32 (
>    IN      UINT32                    Value
>    )
>  {
> -  do {
> -    ((UINT32*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT32*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> @@ -85,9 +85,9 @@ InternalMemSetMem64 (
>    IN      UINT64                    Value
>    )
>  {
> -  do {
> -    ((UINT64*)Buffer)[--Length] = Value;
> -  } while (Length != 0);
> +  for (; Length != 0; Length--) {
> +    ((UINT64*)Buffer)[Length - 1] = Value;
> +  }
>    return Buffer;
>  }
> 
> --
> 1.9.5.msysgit.0

Re: [PATCH 1/6] MdePkg/BaseLib: Refine (Ascii)StrnLenS functions logic

[Wu, Hao A]

Mike,

I have noticed that the patch has the following issue:

According to the comments in functions [Ascii]StrnLens, at most the first
MaxSize characters of String shall be accessed by those two APIs.

But my patch will access (MaxSize + 1) characters of String if there is no
no null character in the first MaxSize characters in String.

Sorry for the issue, I will send out a V2 of the series with this patch
updated.


Best Regards,
Hao Wu


> -----Original Message-----
> From: Kinney, Michael D
> Sent: Friday, December 16, 2016 7:55 AM
> To: Wu, Hao A; edk2-devel@lists.01.org; Kinney, Michael D
> Cc: Wu, Hao A; Yao, Jiewen; Gao, Liming
> Subject: RE: [edk2] [PATCH 1/6] MdePkg/BaseLib: Refine (Ascii)StrnLenS
> functions logic
> 
> Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Hao
> Wu
> > Sent: Wednesday, December 14, 2016 3:27 AM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: [edk2] [PATCH 1/6] MdePkg/BaseLib: Refine (Ascii)StrnLenS functions
> logic
> >
> > This commit refines the logic for AsciiStrnLenS and StrnLenS. It makes the
> > logic more straightforward to prevent possible mis-reports by static code
> > checkers.
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > ---
> >  MdePkg/Library/BaseLib/SafeString.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/MdePkg/Library/BaseLib/SafeString.c
> > b/MdePkg/Library/BaseLib/SafeString.c
> > index ede2f4c..3247d28 100644
> > --- a/MdePkg/Library/BaseLib/SafeString.c
> > +++ b/MdePkg/Library/BaseLib/SafeString.c
> > @@ -143,8 +143,12 @@ StrnLenS (
> >    // String then StrnLenS returns MaxSize. At most the first MaxSize
> characters of
> > String shall
> >    // be accessed by StrnLenS.
> >    //
> > -  for (Length = 0; (Length < MaxSize) && (*String != 0); String++, Length++) {
> > -    ;
> > +  Length = 0;
> > +  while (String[Length] != 0) {
> > +    if (Length >= MaxSize) {
> > +      break;
> > +    }
> > +    Length++;
> >    }
> >    return Length;
> >  }
> > @@ -571,8 +575,12 @@ AsciiStrnLenS (
> >    // String then AsciiStrnLenS returns MaxSize. At most the first MaxSize
> characters
> > of String shall
> >    // be accessed by AsciiStrnLenS.
> >    //
> > -  for (Length = 0; (Length < MaxSize) && (*String != 0); String++, Length++) {
> > -    ;
> > +  Length = 0;
> > +  while (String[Length] != 0) {
> > +    if (Length >= MaxSize) {
> > +      break;
> > +    }
> > +    Length++;
> >    }
> >    return Length;
> >  }
> > --
> > 1.9.5.msysgit.0
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 0/6] Refine code logics to prevent possible mis-reports by static code checkers

[Heyi Guo]

Hi Hao,

May I ask which static code checkers you are using?

We are using Coverity and Fortify checkers, and there are hundreds of 
warnings reported. Do you have a plan to analyze and fix (some may be 
not real errors) the warnings from these two checkers?

Thanks and regards,

Heyi

在 12/14/2016 7:26 PM, Hao Wu 写道:
> The series refines the loop logic (e.g. for, while) of some functions to
> be more straightforward. This will help to prevent some possible
> mis-reports by static code checkers
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
>
> Hao Wu (6):
>    MdePkg/BaseLib: Refine (Ascii)StrnLenS functions logic
>    MdePkg/BaseLib: Add an additional check within (Ascii)StrnCmp
>    MdePkg/MemoryLib: Refine InternalMemSetMem16|32|64 functions logic
>    MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic
>    MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic
>    NetworkPkg: Refine UintnToAscDecWithFormat functions logic
>
>   MdeModulePkg/Library/DxeNetLib/NetBuffer.c             | 16 ++++++++--------
>   .../Universal/Network/UefiPxeBcDxe/PxeBcSupport.c      |  5 ++---
>   MdePkg/Library/BaseLib/SafeString.c                    | 16 ++++++++++++----
>   MdePkg/Library/BaseLib/String.c                        |  4 +++-
>   MdePkg/Library/BaseMemoryLib/MemLibGeneric.c           | 18 +++++++++---------
>   MdePkg/Library/PeiMemoryLib/MemLibGeneric.c            | 18 +++++++++---------
>   MdePkg/Library/UefiMemoryLib/MemLibGeneric.c           | 18 +++++++++---------
>   NetworkPkg/HttpBootDxe/HttpBootSupport.c               |  5 ++---
>   NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c                 |  5 ++---
>   9 files changed, 56 insertions(+), 49 deletions(-)
>

Re: [PATCH 0/6] Refine code logics to prevent possible mis-reports by static code checkers

[Wu, Hao A]

Hi Heyi,

For the warnings reported by the static checkers you mentioned, could you
help to analyze those issues and find out if there are real issues exist?

If there are real code issues, I think you can report them to Bugzilla
with the appropriate classification (e.g. Security or Not Security
Related).

Thanks in advance.

Best Regards,
Hao Wu

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Heyi
> Guo
> Sent: Sunday, December 18, 2016 11:04 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Ye, Ting; Gao, Liming; Wu, Jiaxin; Yao, Jiewen; Kinney, Michael D; Fu, Siyuan
> Subject: Re: [edk2] [PATCH 0/6] Refine code logics to prevent possible mis-
> reports by static code checkers
> 
> Hi Hao,
> 
> May I ask which static code checkers you are using?
> 
> We are using Coverity and Fortify checkers, and there are hundreds of
> warnings reported. Do you have a plan to analyze and fix (some may be
> not real errors) the warnings from these two checkers?
> 
> Thanks and regards,
> 
> Heyi
> 
> 在 12/14/2016 7:26 PM, Hao Wu 写道:
> > The series refines the loop logic (e.g. for, while) of some functions to
> > be more straightforward. This will help to prevent some possible
> > mis-reports by static code checkers
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> >
> > Hao Wu (6):
> >    MdePkg/BaseLib: Refine (Ascii)StrnLenS functions logic
> >    MdePkg/BaseLib: Add an additional check within (Ascii)StrnCmp
> >    MdePkg/MemoryLib: Refine InternalMemSetMem16|32|64 functions logic
> >    MdeModulePkg/DxeNetLib: Rewrite NetblockChecksum function logic
> >    MdeModulePkg/UefiPxeBcDxe: Refine the CvtNum function logic
> >    NetworkPkg: Refine UintnToAscDecWithFormat functions logic
> >
> >   MdeModulePkg/Library/DxeNetLib/NetBuffer.c             | 16 ++++++++--------
> >   .../Universal/Network/UefiPxeBcDxe/PxeBcSupport.c      |  5 ++---
> >   MdePkg/Library/BaseLib/SafeString.c                    | 16 ++++++++++++----
> >   MdePkg/Library/BaseLib/String.c                        |  4 +++-
> >   MdePkg/Library/BaseMemoryLib/MemLibGeneric.c           | 18 +++++++++--
> -------
> >   MdePkg/Library/PeiMemoryLib/MemLibGeneric.c            | 18 +++++++++----
> -----
> >   MdePkg/Library/UefiMemoryLib/MemLibGeneric.c           | 18 +++++++++---
> ------
> >   NetworkPkg/HttpBootDxe/HttpBootSupport.c               |  5 ++---
> >   NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c                 |  5 ++---
> >   9 files changed, 56 insertions(+), 49 deletions(-)
> >
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel