[edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.

Liming Gao posted 1 patch 18 weeks ago
Failed in applying to current master (apply log)
BaseTools/Source/C/BfmLib/BfmLib.c                 | 117 +++++++++++----------
BaseTools/Source/C/FCE/BinaryParse.c               |   2 +-
BaseTools/Source/C/FMMT/FirmwareModuleManagement.c |   2 +-
BaseTools/Source/C/FMMT/FmmtLib.c                  | 117 +++++++++++----------
4 files changed, 122 insertions(+), 116 deletions(-)

[edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.

Posted by Liming Gao 18 weeks ago
From: gaozhic <zhichao.gao@intel.com>

GCC 7 or 8 reports some warnings in new added FCE/FMMT/BlmLib.

Signed-off-by: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
---
In V2: 
  Fix GCC8 compiler issue.

In V3:
  Fix snprintf wrong replacement.

 BaseTools/Source/C/BfmLib/BfmLib.c                 | 117 +++++++++++----------
 BaseTools/Source/C/FCE/BinaryParse.c               |   2 +-
 BaseTools/Source/C/FMMT/FirmwareModuleManagement.c |   2 +-
 BaseTools/Source/C/FMMT/FmmtLib.c                  | 117 +++++++++++----------
 4 files changed, 122 insertions(+), 116 deletions(-)

diff --git a/BaseTools/Source/C/BfmLib/BfmLib.c b/BaseTools/Source/C/BfmLib/BfmLib.c
index 9dedda3da2..8d0ec9038c 100644
--- a/BaseTools/Source/C/BfmLib/BfmLib.c
+++ b/BaseTools/Source/C/BfmLib/BfmLib.c
@@ -9,6 +9,9 @@
 
 #include "BinFileManager.h"
 
+#define STR_LEN_MAX_4K 4096
+#define STR_LEN_MAX_1K 1024
+
 #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes, TestAttributes, Bit) \
     ( \
       (BOOLEAN) ( \
@@ -116,7 +119,7 @@ LibInitializeFvStruct (
 
   for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV; Index ++) {
     memset (Fv->FfsAttuibutes[Index].FfsName, '\0', _MAX_PATH);
-    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
+    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
 
     Fv->FfsAttuibutes[Index].IsLeaf               = TRUE;
     Fv->FfsAttuibutes[Index].TotalSectionNum      = 0;
@@ -2399,153 +2402,153 @@ LibFvHeaderAttributeToStr (
 
   LocalStr  = NULL;
 
-  LocalStr = (CHAR8 *) malloc (1024 * 4);
+  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
 
   if (LocalStr == NULL) {
     printf ("Out of resource, memory allocation failed. \n");
     return EFI_OUT_OF_RESOURCES;
   }
 
-  memset (LocalStr, '\0', 1024 * 4);
+  memset (LocalStr, '\0', STR_LEN_MAX_4K);
 
   if (Attr == 0 || InfFile  == NULL) {
     free (LocalStr);
     return EFI_INVALID_PARAMETER;
   }
 
-  strncat (LocalStr, "[attributes] \n", sizeof("[attributes] \n"));
+  strncat (LocalStr, "[attributes] \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
 
   if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
-    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
-    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_READ_STATUS) {
-    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", sizeof ("EFI_READ_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
-    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
-    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_STATUS) {
-    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", sizeof ("EFI_WRITE_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_LOCK_CAP) {
-    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", sizeof ("EFI_LOCK_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_LOCK_STATUS) {
-    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", sizeof ("EFI_LOCK_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_STICKY_WRITE) {
-    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", sizeof ("EFI_STICKY_WRITE = TRUE \n"));
+    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_MEMORY_MAPPED) {
-    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
+    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_ERASE_POLARITY) {
-    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", sizeof ("EFI_ERASE_POLARITY = 1 \n"));
+    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_READ_LOCK_CAP) {
-    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
-    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
-    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
-    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_LOCK_STATUS) {
-    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   //
   // Alignment
   //
   if (Attr & EFI_FVB2_ALIGNMENT_1) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
@@ -2592,7 +2595,7 @@ LibFvHeaderOptionToStr (
   //
   // This section will not over 1024 bytes and each line will never over 128 bytes.
   //
-  LocalStr    = (CHAR8 *) malloc (1024);
+  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
   BlockSize   = (CHAR8 *) malloc (128);
   NumOfBlocks = (CHAR8 *) malloc (128);
 
@@ -2611,18 +2614,18 @@ LibFvHeaderOptionToStr (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  memset (LocalStr, '\0', 1024);
+  memset (LocalStr, '\0', STR_LEN_MAX_1K);
   memset (BlockSize, '\0', 128);
   memset (NumOfBlocks, '\0', 128);
 
-  strncat (LocalStr, "[options] \n", sizeof("[Options] \n"));
+  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K - strlen (LocalStr) - 1);
 
   sprintf (BlockSize, "EFI_BLOCK_SIZE  = 0x%x \n", BlockMap->Length);
-  strncat (LocalStr, BlockSize, strlen(BlockSize));
+  strncat (LocalStr, BlockSize, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
 
   if (IsRootFv) {
   sprintf (NumOfBlocks, "EFI_NUM_BLOCKS  = 0x%x \n", BlockMap->NumBlocks);
-  strncat (LocalStr, NumOfBlocks, strlen(NumOfBlocks));
+  strncat (LocalStr, NumOfBlocks, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
   }
 
   if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
diff --git a/BaseTools/Source/C/FCE/BinaryParse.c b/BaseTools/Source/C/FCE/BinaryParse.c
index e9f8ee6826..97d6ecf93b 100644
--- a/BaseTools/Source/C/FCE/BinaryParse.c
+++ b/BaseTools/Source/C/FCE/BinaryParse.c
@@ -1240,7 +1240,7 @@ Done:
       ) {
         continue;
       }
-      sprintf (FileNameArry, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
+      snprintf (FileNameArry, MAX_FILENAME_LEN, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
       FfsFile = fopen (FileNameArry, "rb");
       Status = ReadFfsHeader (FfsFile, (UINT32 *)&FileSize);
       if (EFI_ERROR (Status)) {
diff --git a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
index 63ae3c45a4..6648fbd54f 100644
--- a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
+++ b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
@@ -1572,7 +1572,7 @@ FmmtImageDelete (
                     // If decrease operation executed, we should adjust the ffs list. It will bring in more complex.
                     //
                     //FvInFd->FfsNumbers                    -= 1;
-                    memset(FvInFd->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
+                    memset(FvInFd->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
                    if (FvInFd->FfsAttuibutes[Index].FvLevel > 1) {
                        for (j = Index - 1; j >= 0; j--) {
                            if (FvInFd->FfsAttuibutes[j].FvLevel == FvInFd->FfsAttuibutes[Index].FvLevel - 1) {
diff --git a/BaseTools/Source/C/FMMT/FmmtLib.c b/BaseTools/Source/C/FMMT/FmmtLib.c
index f87042114b..e236810c59 100644
--- a/BaseTools/Source/C/FMMT/FmmtLib.c
+++ b/BaseTools/Source/C/FMMT/FmmtLib.c
@@ -9,6 +9,9 @@
 
 #include "FirmwareModuleManagement.h"
 
+#define STR_LEN_MAX_4K 4096
+#define STR_LEN_MAX_1K 1024
+
 #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes, TestAttributes, Bit) \
     ( \
       (BOOLEAN) ( \
@@ -155,7 +158,7 @@ LibInitializeFvStruct (
 
   for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV; Index ++) {
     memset (Fv->FfsAttuibutes[Index].FfsName, '\0', _MAX_PATH);
-    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
+    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
     memset (&Fv->FfsAttuibutes[Index].GuidName, '\0', sizeof(EFI_GUID));
     Fv->FfsAttuibutes[Index].UiNameSize           = 0;
     Fv->FfsAttuibutes[Index].IsLeaf               = TRUE;
@@ -2504,153 +2507,153 @@ LibFvHeaderAttributeToStr (
 
   LocalStr  = NULL;
 
-  LocalStr = (CHAR8 *) malloc (1024 * 4);
+  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
 
   if (LocalStr == NULL) {
     printf ("Memory allocate error!\n");
     return EFI_OUT_OF_RESOURCES;
   }
 
-  memset (LocalStr, '\0', 1024 * 4);
+  memset (LocalStr, '\0', STR_LEN_MAX_4K);
 
   if (Attr == 0 || InfFile  == NULL) {
     free (LocalStr);
     return EFI_INVALID_PARAMETER;
   }
 
-  strncat (LocalStr, "[attributes] \n", sizeof("[attributes] \n"));
+  strncat (LocalStr, "[attributes] \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
 
   if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
-    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
-    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_READ_STATUS) {
-    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", sizeof ("EFI_READ_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
-    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
-    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_STATUS) {
-    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", sizeof ("EFI_WRITE_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_LOCK_CAP) {
-    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", sizeof ("EFI_LOCK_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_LOCK_STATUS) {
-    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", sizeof ("EFI_LOCK_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_STICKY_WRITE) {
-    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", sizeof ("EFI_STICKY_WRITE = TRUE \n"));
+    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_MEMORY_MAPPED) {
-    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
+    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_ERASE_POLARITY) {
-    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", sizeof ("EFI_ERASE_POLARITY = 1 \n"));
+    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_READ_LOCK_CAP) {
-    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
-    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
-    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
-    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (Attr & EFI_FVB2_LOCK_STATUS) {
-    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
+    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   //
   // Alignment
   //
   if (Attr & EFI_FVB2_ALIGNMENT_1) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
-    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
+    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
   }
 
   if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
@@ -2696,7 +2699,7 @@ LibFvHeaderOptionToStr (
   //
   // This section will not over 1024 bytes and each line will never over 128 bytes.
   //
-  LocalStr    = (CHAR8 *) malloc (1024);
+  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
   TempStr     = (CHAR8 *) malloc (128);
 
   if (LocalStr    == NULL ||
@@ -2712,18 +2715,18 @@ LibFvHeaderOptionToStr (
   }
 
   BlockMap = FvHeader->BlockMap;
-  memset (LocalStr, '\0', 1024);
+  memset (LocalStr, '\0', STR_LEN_MAX_1K);
   memset (TempStr, '\0', 128);
 
-  strncat (LocalStr, "[options] \n", sizeof("[Options] \n"));
+  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K - strlen (LocalStr) - 1);
 
 
   snprintf (TempStr, 128, "EFI_BLOCK_SIZE  = 0x%x \n", BlockMap->Length);
-  strncat (LocalStr, TempStr, strlen(TempStr));
+  strncat (LocalStr, TempStr, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
 
   if (IsRootFv) {
   snprintf (TempStr, 128, "EFI_NUM_BLOCKS  = 0x%x \n", BlockMap->NumBlocks);
-  strncat (LocalStr, TempStr, strlen(TempStr));
+  strncat (LocalStr, TempStr, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
   }
 
   if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
-- 
2.13.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43435): https://edk2.groups.io/g/devel/message/43435
Mute This Topic: https://groups.io/mt/32403705/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.

Posted by Leif Lindholm 18 weeks ago
Hi Liming,

On Tue, Jul 09, 2019 at 05:53:33PM +0800, Liming Gao wrote:
> From: gaozhic <zhichao.gao@intel.com>
> 
> GCC 7 or 8 reports some warnings in new added FCE/FMMT/BlmLib.

Please list the specific warnings addressed.
(But also see my comments below - I think this utility needs ripping
out and rewriting.)

> Signed-off-by: Liming Gao <liming.gao@intel.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> ---
> In V2: 
>   Fix GCC8 compiler issue.
> 
> In V3:
>   Fix snprintf wrong replacement.
> 
>  BaseTools/Source/C/BfmLib/BfmLib.c                 | 117 +++++++++++----------
>  BaseTools/Source/C/FCE/BinaryParse.c               |   2 +-
>  BaseTools/Source/C/FMMT/FirmwareModuleManagement.c |   2 +-
>  BaseTools/Source/C/FMMT/FmmtLib.c                  | 117 +++++++++++----------
>  4 files changed, 122 insertions(+), 116 deletions(-)
> 
> diff --git a/BaseTools/Source/C/BfmLib/BfmLib.c b/BaseTools/Source/C/BfmLib/BfmLib.c
> index 9dedda3da2..8d0ec9038c 100644
> --- a/BaseTools/Source/C/BfmLib/BfmLib.c
> +++ b/BaseTools/Source/C/BfmLib/BfmLib.c
> @@ -9,6 +9,9 @@
>  
>  #include "BinFileManager.h"
>  
> +#define STR_LEN_MAX_4K 4096
> +#define STR_LEN_MAX_1K 1024
> +

Coding style 3.3.3: "Code files should not contain #define and typedef
statements."

Move these to BinFileManager.h?

>  #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes, TestAttributes, Bit) \
>      ( \
>        (BOOLEAN) ( \
> @@ -116,7 +119,7 @@ LibInitializeFvStruct (
>  
>    for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV; Index ++) {
>      memset (Fv->FfsAttuibutes[Index].FfsName, '\0', _MAX_PATH);
> -    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
> +    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
>  
>      Fv->FfsAttuibutes[Index].IsLeaf               = TRUE;
>      Fv->FfsAttuibutes[Index].TotalSectionNum      = 0;
> @@ -2399,153 +2402,153 @@ LibFvHeaderAttributeToStr (
>  
>    LocalStr  = NULL;
>  
> -  LocalStr = (CHAR8 *) malloc (1024 * 4);
> +  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
>  
>    if (LocalStr == NULL) {
>      printf ("Out of resource, memory allocation failed. \n");
>      return EFI_OUT_OF_RESOURCES;
>    }
>  
> -  memset (LocalStr, '\0', 1024 * 4);
> +  memset (LocalStr, '\0', STR_LEN_MAX_4K);
>  
>    if (Attr == 0 || InfFile  == NULL) {
>      free (LocalStr);
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  strncat (LocalStr, "[attributes] \n", sizeof("[attributes] \n"));
> +  strncat (LocalStr, "[attributes] \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);

This is a very inefficient, and difficult to read, way of doing this.
It also performs absolutely no error checking (making any future
debugging tedious at best).

I have to be honest - looking at this code, I think the right thing to
do would be to revert the commit adding it and reworking the utility
completely (making sure it gets serious review) before merging it.

I am not surprised the compilers get upset.

This made me have a closer look at the patches adding FCE and FMMT,
and frankly, my opinion of them are similar. These aren't being
upstreamed - this is textbook "chuck over the wall".

Don't get me wrong - the code quality of FMMT is notably higher than
the code quality of FCE, which is notably higher than the code quality
of BfmLib. But even 

BfmLib was added with the comment that it is used by FCE.
So why do FCE and FMMT both have separate copies of LibBfmGuidToStr?
Not just functions doing the same thing, but with the same name.

There are many other issues with these tools.

/
    Leif

>  
>    if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
> -    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
> -    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_READ_STATUS) {
> -    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", sizeof ("EFI_READ_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
> -    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
> -    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_STATUS) {
> -    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", sizeof ("EFI_WRITE_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_LOCK_CAP) {
> -    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", sizeof ("EFI_LOCK_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_LOCK_STATUS) {
> -    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", sizeof ("EFI_LOCK_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_STICKY_WRITE) {
> -    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", sizeof ("EFI_STICKY_WRITE = TRUE \n"));
> +    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_MEMORY_MAPPED) {
> -    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
> +    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_ERASE_POLARITY) {
> -    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", sizeof ("EFI_ERASE_POLARITY = 1 \n"));
> +    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_READ_LOCK_CAP) {
> -    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
> -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
> -    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
> -    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_LOCK_STATUS) {
> -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    //
>    // Alignment
>    //
>    if (Attr & EFI_FVB2_ALIGNMENT_1) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
> @@ -2592,7 +2595,7 @@ LibFvHeaderOptionToStr (
>    //
>    // This section will not over 1024 bytes and each line will never over 128 bytes.
>    //
> -  LocalStr    = (CHAR8 *) malloc (1024);
> +  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
>    BlockSize   = (CHAR8 *) malloc (128);
>    NumOfBlocks = (CHAR8 *) malloc (128);
>  
> @@ -2611,18 +2614,18 @@ LibFvHeaderOptionToStr (
>      return EFI_OUT_OF_RESOURCES;
>    }
>  
> -  memset (LocalStr, '\0', 1024);
> +  memset (LocalStr, '\0', STR_LEN_MAX_1K);
>    memset (BlockSize, '\0', 128);
>    memset (NumOfBlocks, '\0', 128);
>  
> -  strncat (LocalStr, "[options] \n", sizeof("[Options] \n"));
> +  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K - strlen (LocalStr) - 1);
>  
>    sprintf (BlockSize, "EFI_BLOCK_SIZE  = 0x%x \n", BlockMap->Length);
> -  strncat (LocalStr, BlockSize, strlen(BlockSize));
> +  strncat (LocalStr, BlockSize, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
>  
>    if (IsRootFv) {
>    sprintf (NumOfBlocks, "EFI_NUM_BLOCKS  = 0x%x \n", BlockMap->NumBlocks);
> -  strncat (LocalStr, NumOfBlocks, strlen(NumOfBlocks));
> +  strncat (LocalStr, NumOfBlocks, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
>    }
>  
>    if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
> diff --git a/BaseTools/Source/C/FCE/BinaryParse.c b/BaseTools/Source/C/FCE/BinaryParse.c
> index e9f8ee6826..97d6ecf93b 100644
> --- a/BaseTools/Source/C/FCE/BinaryParse.c
> +++ b/BaseTools/Source/C/FCE/BinaryParse.c
> @@ -1240,7 +1240,7 @@ Done:
>        ) {
>          continue;
>        }
> -      sprintf (FileNameArry, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
> +      snprintf (FileNameArry, MAX_FILENAME_LEN, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
>        FfsFile = fopen (FileNameArry, "rb");
>        Status = ReadFfsHeader (FfsFile, (UINT32 *)&FileSize);
>        if (EFI_ERROR (Status)) {
> diff --git a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> index 63ae3c45a4..6648fbd54f 100644
> --- a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> +++ b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> @@ -1572,7 +1572,7 @@ FmmtImageDelete (
>                      // If decrease operation executed, we should adjust the ffs list. It will bring in more complex.
>                      //
>                      //FvInFd->FfsNumbers                    -= 1;
> -                    memset(FvInFd->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
> +                    memset(FvInFd->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
>                     if (FvInFd->FfsAttuibutes[Index].FvLevel > 1) {
>                         for (j = Index - 1; j >= 0; j--) {
>                             if (FvInFd->FfsAttuibutes[j].FvLevel == FvInFd->FfsAttuibutes[Index].FvLevel - 1) {
> diff --git a/BaseTools/Source/C/FMMT/FmmtLib.c b/BaseTools/Source/C/FMMT/FmmtLib.c
> index f87042114b..e236810c59 100644
> --- a/BaseTools/Source/C/FMMT/FmmtLib.c
> +++ b/BaseTools/Source/C/FMMT/FmmtLib.c
> @@ -9,6 +9,9 @@
>  
>  #include "FirmwareModuleManagement.h"
>  
> +#define STR_LEN_MAX_4K 4096
> +#define STR_LEN_MAX_1K 1024
> +
>  #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes, TestAttributes, Bit) \
>      ( \
>        (BOOLEAN) ( \
> @@ -155,7 +158,7 @@ LibInitializeFvStruct (
>  
>    for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV; Index ++) {
>      memset (Fv->FfsAttuibutes[Index].FfsName, '\0', _MAX_PATH);
> -    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
> +    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
>      memset (&Fv->FfsAttuibutes[Index].GuidName, '\0', sizeof(EFI_GUID));
>      Fv->FfsAttuibutes[Index].UiNameSize           = 0;
>      Fv->FfsAttuibutes[Index].IsLeaf               = TRUE;
> @@ -2504,153 +2507,153 @@ LibFvHeaderAttributeToStr (
>  
>    LocalStr  = NULL;
>  
> -  LocalStr = (CHAR8 *) malloc (1024 * 4);
> +  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
>  
>    if (LocalStr == NULL) {
>      printf ("Memory allocate error!\n");
>      return EFI_OUT_OF_RESOURCES;
>    }
>  
> -  memset (LocalStr, '\0', 1024 * 4);
> +  memset (LocalStr, '\0', STR_LEN_MAX_4K);
>  
>    if (Attr == 0 || InfFile  == NULL) {
>      free (LocalStr);
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  strncat (LocalStr, "[attributes] \n", sizeof("[attributes] \n"));
> +  strncat (LocalStr, "[attributes] \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>  
>    if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
> -    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
> -    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_READ_STATUS) {
> -    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", sizeof ("EFI_READ_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
> -    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
> -    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_STATUS) {
> -    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", sizeof ("EFI_WRITE_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_LOCK_CAP) {
> -    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", sizeof ("EFI_LOCK_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_LOCK_STATUS) {
> -    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", sizeof ("EFI_LOCK_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_STICKY_WRITE) {
> -    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", sizeof ("EFI_STICKY_WRITE = TRUE \n"));
> +    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_MEMORY_MAPPED) {
> -    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
> +    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_ERASE_POLARITY) {
> -    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", sizeof ("EFI_ERASE_POLARITY = 1 \n"));
> +    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_READ_LOCK_CAP) {
> -    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
> -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
> -    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
> -    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (Attr & EFI_FVB2_LOCK_STATUS) {
> -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    //
>    // Alignment
>    //
>    if (Attr & EFI_FVB2_ALIGNMENT_1) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
> -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
> +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>    }
>  
>    if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
> @@ -2696,7 +2699,7 @@ LibFvHeaderOptionToStr (
>    //
>    // This section will not over 1024 bytes and each line will never over 128 bytes.
>    //
> -  LocalStr    = (CHAR8 *) malloc (1024);
> +  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
>    TempStr     = (CHAR8 *) malloc (128);
>  
>    if (LocalStr    == NULL ||
> @@ -2712,18 +2715,18 @@ LibFvHeaderOptionToStr (
>    }
>  
>    BlockMap = FvHeader->BlockMap;
> -  memset (LocalStr, '\0', 1024);
> +  memset (LocalStr, '\0', STR_LEN_MAX_1K);
>    memset (TempStr, '\0', 128);
>  
> -  strncat (LocalStr, "[options] \n", sizeof("[Options] \n"));
> +  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K - strlen (LocalStr) - 1);
>  
>  
>    snprintf (TempStr, 128, "EFI_BLOCK_SIZE  = 0x%x \n", BlockMap->Length);
> -  strncat (LocalStr, TempStr, strlen(TempStr));
> +  strncat (LocalStr, TempStr, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
>  
>    if (IsRootFv) {
>    snprintf (TempStr, 128, "EFI_NUM_BLOCKS  = 0x%x \n", BlockMap->NumBlocks);
> -  strncat (LocalStr, TempStr, strlen(TempStr));
> +  strncat (LocalStr, TempStr, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
>    }
>  
>    if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
> -- 
> 2.13.0.windows.1
> 
> 
> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43447): https://edk2.groups.io/g/devel/message/43447
Mute This Topic: https://groups.io/mt/32403705/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.

Posted by Liming Gao 18 weeks ago
Lefi:

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Leif Lindholm
> Sent: Wednesday, July 10, 2019 2:26 AM
> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
> Cc: Gao, Zhichao <zhichao.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Andrew Fish <afish@apple.com>; Laszlo Ersek
> <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.
> 
> Hi Liming,
> 
> On Tue, Jul 09, 2019 at 05:53:33PM +0800, Liming Gao wrote:
> > From: gaozhic <zhichao.gao@intel.com>
> >
> > GCC 7 or 8 reports some warnings in new added FCE/FMMT/BlmLib.
> 
> Please list the specific warnings addressed.
> (But also see my comments below - I think this utility needs ripping
> out and rewriting.)
> 
> > Signed-off-by: Liming Gao <liming.gao@intel.com>
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > ---
> > In V2:
> >   Fix GCC8 compiler issue.
> >
> > In V3:
> >   Fix snprintf wrong replacement.
> >
> >  BaseTools/Source/C/BfmLib/BfmLib.c                 | 117 +++++++++++----------
> >  BaseTools/Source/C/FCE/BinaryParse.c               |   2 +-
> >  BaseTools/Source/C/FMMT/FirmwareModuleManagement.c |   2 +-
> >  BaseTools/Source/C/FMMT/FmmtLib.c                  | 117 +++++++++++----------
> >  4 files changed, 122 insertions(+), 116 deletions(-)
> >
> > diff --git a/BaseTools/Source/C/BfmLib/BfmLib.c b/BaseTools/Source/C/BfmLib/BfmLib.c
> > index 9dedda3da2..8d0ec9038c 100644
> > --- a/BaseTools/Source/C/BfmLib/BfmLib.c
> > +++ b/BaseTools/Source/C/BfmLib/BfmLib.c
> > @@ -9,6 +9,9 @@
> >
> >  #include "BinFileManager.h"
> >
> > +#define STR_LEN_MAX_4K 4096
> > +#define STR_LEN_MAX_1K 1024
> > +
> 
> Coding style 3.3.3: "Code files should not contain #define and typedef
> statements."
> 
> Move these to BinFileManager.h?
> 
> >  #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes, TestAttributes, Bit) \
> >      ( \
> >        (BOOLEAN) ( \
> > @@ -116,7 +119,7 @@ LibInitializeFvStruct (
> >
> >    for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV; Index ++) {
> >      memset (Fv->FfsAttuibutes[Index].FfsName, '\0', _MAX_PATH);
> > -    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
> > +    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
> >
> >      Fv->FfsAttuibutes[Index].IsLeaf               = TRUE;
> >      Fv->FfsAttuibutes[Index].TotalSectionNum      = 0;
> > @@ -2399,153 +2402,153 @@ LibFvHeaderAttributeToStr (
> >
> >    LocalStr  = NULL;
> >
> > -  LocalStr = (CHAR8 *) malloc (1024 * 4);
> > +  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
> >
> >    if (LocalStr == NULL) {
> >      printf ("Out of resource, memory allocation failed. \n");
> >      return EFI_OUT_OF_RESOURCES;
> >    }
> >
> > -  memset (LocalStr, '\0', 1024 * 4);
> > +  memset (LocalStr, '\0', STR_LEN_MAX_4K);
> >
> >    if (Attr == 0 || InfFile  == NULL) {
> >      free (LocalStr);
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > -  strncat (LocalStr, "[attributes] \n", sizeof("[attributes] \n"));
> > +  strncat (LocalStr, "[attributes] \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> 
> This is a very inefficient, and difficult to read, way of doing this.
> It also performs absolutely no error checking (making any future
> debugging tedious at best).
> 
This change is to fix gcc warning -Werror=sizeof-pointer-memaccess.
strncat is the safe version API. So, it is used here.

> I have to be honest - looking at this code, I think the right thing to
> do would be to revert the commit adding it and reworking the utility
> completely (making sure it gets serious review) before merging it.
> 
> I am not surprised the compilers get upset.
> 
> This made me have a closer look at the patches adding FCE and FMMT,
> and frankly, my opinion of them are similar. These aren't being
> upstreamed - this is textbook "chuck over the wall".
> 
> Don't get me wrong - the code quality of FMMT is notably higher than
> the code quality of FCE, which is notably higher than the code quality
> of BfmLib. But even
> 
> BfmLib was added with the comment that it is used by FCE.
> So why do FCE and FMMT both have separate copies of LibBfmGuidToStr?
> Not just functions doing the same thing, but with the same name.
> 
> There are many other issues with these tools.

FCE & FMMT both operate the binary FV image. Their implementation may copy the same logic. I don't think this is a good way.
I agree to do better design for code sharing and code quality improvement. 

I suggest to revert them, add current FCE & FMMT into edk2-staging, then refine them, and send the patch to add them back into edk2 BaseTools.

Thanks
Liming
> 
> /
>     Leif
> 
> >
> >    if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
> > -    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
> > -    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_READ_STATUS) {
> > -    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", sizeof ("EFI_READ_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
> > -    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
> > -    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_STATUS) {
> > -    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", sizeof ("EFI_WRITE_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_LOCK_CAP) {
> > -    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", sizeof ("EFI_LOCK_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_LOCK_STATUS) {
> > -    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", sizeof ("EFI_LOCK_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_STICKY_WRITE) {
> > -    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", sizeof ("EFI_STICKY_WRITE = TRUE \n"));
> > +    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_MEMORY_MAPPED) {
> > -    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
> > +    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_ERASE_POLARITY) {
> > -    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", sizeof ("EFI_ERASE_POLARITY = 1 \n"));
> > +    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_READ_LOCK_CAP) {
> > -    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
> > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
> > -    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
> > -    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_LOCK_STATUS) {
> > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    //
> >    // Alignment
> >    //
> >    if (Attr & EFI_FVB2_ALIGNMENT_1) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
> > @@ -2592,7 +2595,7 @@ LibFvHeaderOptionToStr (
> >    //
> >    // This section will not over 1024 bytes and each line will never over 128 bytes.
> >    //
> > -  LocalStr    = (CHAR8 *) malloc (1024);
> > +  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
> >    BlockSize   = (CHAR8 *) malloc (128);
> >    NumOfBlocks = (CHAR8 *) malloc (128);
> >
> > @@ -2611,18 +2614,18 @@ LibFvHeaderOptionToStr (
> >      return EFI_OUT_OF_RESOURCES;
> >    }
> >
> > -  memset (LocalStr, '\0', 1024);
> > +  memset (LocalStr, '\0', STR_LEN_MAX_1K);
> >    memset (BlockSize, '\0', 128);
> >    memset (NumOfBlocks, '\0', 128);
> >
> > -  strncat (LocalStr, "[options] \n", sizeof("[Options] \n"));
> > +  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K - strlen (LocalStr) - 1);
> >
> >    sprintf (BlockSize, "EFI_BLOCK_SIZE  = 0x%x \n", BlockMap->Length);
> > -  strncat (LocalStr, BlockSize, strlen(BlockSize));
> > +  strncat (LocalStr, BlockSize, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
> >
> >    if (IsRootFv) {
> >    sprintf (NumOfBlocks, "EFI_NUM_BLOCKS  = 0x%x \n", BlockMap->NumBlocks);
> > -  strncat (LocalStr, NumOfBlocks, strlen(NumOfBlocks));
> > +  strncat (LocalStr, NumOfBlocks, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
> > diff --git a/BaseTools/Source/C/FCE/BinaryParse.c b/BaseTools/Source/C/FCE/BinaryParse.c
> > index e9f8ee6826..97d6ecf93b 100644
> > --- a/BaseTools/Source/C/FCE/BinaryParse.c
> > +++ b/BaseTools/Source/C/FCE/BinaryParse.c
> > @@ -1240,7 +1240,7 @@ Done:
> >        ) {
> >          continue;
> >        }
> > -      sprintf (FileNameArry, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
> > +      snprintf (FileNameArry, MAX_FILENAME_LEN, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
> >        FfsFile = fopen (FileNameArry, "rb");
> >        Status = ReadFfsHeader (FfsFile, (UINT32 *)&FileSize);
> >        if (EFI_ERROR (Status)) {
> > diff --git a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> > index 63ae3c45a4..6648fbd54f 100644
> > --- a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> > +++ b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> > @@ -1572,7 +1572,7 @@ FmmtImageDelete (
> >                      // If decrease operation executed, we should adjust the ffs list. It will bring in more complex.
> >                      //
> >                      //FvInFd->FfsNumbers                    -= 1;
> > -                    memset(FvInFd->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
> > +                    memset(FvInFd->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
> >                     if (FvInFd->FfsAttuibutes[Index].FvLevel > 1) {
> >                         for (j = Index - 1; j >= 0; j--) {
> >                             if (FvInFd->FfsAttuibutes[j].FvLevel == FvInFd->FfsAttuibutes[Index].FvLevel - 1) {
> > diff --git a/BaseTools/Source/C/FMMT/FmmtLib.c b/BaseTools/Source/C/FMMT/FmmtLib.c
> > index f87042114b..e236810c59 100644
> > --- a/BaseTools/Source/C/FMMT/FmmtLib.c
> > +++ b/BaseTools/Source/C/FMMT/FmmtLib.c
> > @@ -9,6 +9,9 @@
> >
> >  #include "FirmwareModuleManagement.h"
> >
> > +#define STR_LEN_MAX_4K 4096
> > +#define STR_LEN_MAX_1K 1024
> > +
> >  #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes, TestAttributes, Bit) \
> >      ( \
> >        (BOOLEAN) ( \
> > @@ -155,7 +158,7 @@ LibInitializeFvStruct (
> >
> >    for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV; Index ++) {
> >      memset (Fv->FfsAttuibutes[Index].FfsName, '\0', _MAX_PATH);
> > -    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
> > +    memset (Fv->FfsAttuibutes[Index].UiName, '\0', _MAX_PATH * sizeof (CHAR16));
> >      memset (&Fv->FfsAttuibutes[Index].GuidName, '\0', sizeof(EFI_GUID));
> >      Fv->FfsAttuibutes[Index].UiNameSize           = 0;
> >      Fv->FfsAttuibutes[Index].IsLeaf               = TRUE;
> > @@ -2504,153 +2507,153 @@ LibFvHeaderAttributeToStr (
> >
> >    LocalStr  = NULL;
> >
> > -  LocalStr = (CHAR8 *) malloc (1024 * 4);
> > +  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
> >
> >    if (LocalStr == NULL) {
> >      printf ("Memory allocate error!\n");
> >      return EFI_OUT_OF_RESOURCES;
> >    }
> >
> > -  memset (LocalStr, '\0', 1024 * 4);
> > +  memset (LocalStr, '\0', STR_LEN_MAX_4K);
> >
> >    if (Attr == 0 || InfFile  == NULL) {
> >      free (LocalStr);
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > -  strncat (LocalStr, "[attributes] \n", sizeof("[attributes] \n"));
> > +  strncat (LocalStr, "[attributes] \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >
> >    if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
> > -    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
> > -    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_READ_STATUS) {
> > -    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", sizeof ("EFI_READ_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
> > -    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
> > -    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_STATUS) {
> > -    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", sizeof ("EFI_WRITE_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_LOCK_CAP) {
> > -    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", sizeof ("EFI_LOCK_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_LOCK_STATUS) {
> > -    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", sizeof ("EFI_LOCK_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_STICKY_WRITE) {
> > -    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", sizeof ("EFI_STICKY_WRITE = TRUE \n"));
> > +    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_MEMORY_MAPPED) {
> > -    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
> > +    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_ERASE_POLARITY) {
> > -    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", sizeof ("EFI_ERASE_POLARITY = 1 \n"));
> > +    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_READ_LOCK_CAP) {
> > -    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
> > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
> > -    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
> > -    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (Attr & EFI_FVB2_LOCK_STATUS) {
> > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    //
> >    // Alignment
> >    //
> >    if (Attr & EFI_FVB2_ALIGNMENT_1) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
> > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
> > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
> > @@ -2696,7 +2699,7 @@ LibFvHeaderOptionToStr (
> >    //
> >    // This section will not over 1024 bytes and each line will never over 128 bytes.
> >    //
> > -  LocalStr    = (CHAR8 *) malloc (1024);
> > +  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
> >    TempStr     = (CHAR8 *) malloc (128);
> >
> >    if (LocalStr    == NULL ||
> > @@ -2712,18 +2715,18 @@ LibFvHeaderOptionToStr (
> >    }
> >
> >    BlockMap = FvHeader->BlockMap;
> > -  memset (LocalStr, '\0', 1024);
> > +  memset (LocalStr, '\0', STR_LEN_MAX_1K);
> >    memset (TempStr, '\0', 128);
> >
> > -  strncat (LocalStr, "[options] \n", sizeof("[Options] \n"));
> > +  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K - strlen (LocalStr) - 1);
> >
> >
> >    snprintf (TempStr, 128, "EFI_BLOCK_SIZE  = 0x%x \n", BlockMap->Length);
> > -  strncat (LocalStr, TempStr, strlen(TempStr));
> > +  strncat (LocalStr, TempStr, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
> >
> >    if (IsRootFv) {
> >    snprintf (TempStr, 128, "EFI_NUM_BLOCKS  = 0x%x \n", BlockMap->NumBlocks);
> > -  strncat (LocalStr, TempStr, strlen(TempStr));
> > +  strncat (LocalStr, TempStr, STR_LEN_MAX_1K - strlen (LocalStr) - 1);
> >    }
> >
> >    if (fwrite (LocalStr, 1, (size_t) strlen (LocalStr), InfFile) != (size_t) strlen (LocalStr)) {
> > --
> > 2.13.0.windows.1
> >
> >
> >
> >
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43479): https://edk2.groups.io/g/devel/message/43479
Mute This Topic: https://groups.io/mt/32403705/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.

Posted by Leif Lindholm 18 weeks ago
On Wed, Jul 10, 2019 at 01:42:44PM +0000, Liming Gao wrote:
> > > -  strncat (LocalStr, "[attributes] \n", sizeof("[attributes] \n"));
> > > +  strncat (LocalStr, "[attributes] \n", STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > 
> > This is a very inefficient, and difficult to read, way of doing this.
> > It also performs absolutely no error checking (making any future
> > debugging tedious at best).
> 
> This change is to fix gcc warning -Werror=sizeof-pointer-memaccess.
> strncat is the safe version API. So, it is used here.

Yes, but this code is continuously appending to a long string, and
then counting the length of that string. This could be done *very*
much more efficiently by keeping track of the current position we're
appending to and using strncpy. It would also improve readability.

As for the safe version - my point about error checking was not to do
with buffer overruns. It was to do with the fact that if the code ends
up filling the buffer, that will go undetected until something goes
wrong further along.

> > I have to be honest - looking at this code, I think the right thing to
> > do would be to revert the commit adding it and reworking the utility
> > completely (making sure it gets serious review) before merging it.
> > 
> > I am not surprised the compilers get upset.
> > 
> > This made me have a closer look at the patches adding FCE and FMMT,
> > and frankly, my opinion of them are similar. These aren't being
> > upstreamed - this is textbook "chuck over the wall".
> > 
> > Don't get me wrong - the code quality of FMMT is notably higher than
> > the code quality of FCE, which is notably higher than the code quality
> > of BfmLib. But even
> > 
> > BfmLib was added with the comment that it is used by FCE.
> > So why do FCE and FMMT both have separate copies of LibBfmGuidToStr?
> > Not just functions doing the same thing, but with the same name.
> > 
> > There are many other issues with these tools.
> 
> FCE & FMMT both operate the binary FV image. Their implementation
> may copy the same logic. I don't think this is a good way.
> I agree to do better design for code sharing and code quality improvement. 
> 
> I suggest to revert them, add current FCE & FMMT into edk2-staging,
> then refine them, and send the patch to add them back into edk2
> BaseTools.

This sounds good to me - thanks!

Best Regards,

Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43527): https://edk2.groups.io/g/devel/message/43527
Mute This Topic: https://groups.io/mt/32403705/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.

Posted by Michael D Kinney 18 weeks ago
Liming,

I agree that edk2-staging is a good place to work on cleaning
these tools up.

Please revert the commit of these tools today so GCC builds
are not broken.

Thanks,

Mike

> -----Original Message-----
> From: Gao, Liming
> Sent: Wednesday, July 10, 2019 6:43 AM
> To: devel@edk2.groups.io; leif.lindholm@linaro.org
> Cc: Gao, Zhichao <zhichao.gao@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>; Andrew Fish <afish@apple.com>;
> Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [Patch v3] BaseTools: Fix GCC
> compiler failure in new added tools.
> 
> Lefi:
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of
> > Leif Lindholm
> > Sent: Wednesday, July 10, 2019 2:26 AM
> > To: devel@edk2.groups.io; Gao, Liming
> <liming.gao@intel.com>
> > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Feng, Bob C
> > <bob.c.feng@intel.com>; Andrew Fish <afish@apple.com>;
> Laszlo Ersek
> > <lersek@redhat.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC
> compiler failure in new added tools.
> >
> > Hi Liming,
> >
> > On Tue, Jul 09, 2019 at 05:53:33PM +0800, Liming Gao
> wrote:
> > > From: gaozhic <zhichao.gao@intel.com>
> > >
> > > GCC 7 or 8 reports some warnings in new added
> FCE/FMMT/BlmLib.
> >
> > Please list the specific warnings addressed.
> > (But also see my comments below - I think this utility
> needs ripping
> > out and rewriting.)
> >
> > > Signed-off-by: Liming Gao <liming.gao@intel.com>
> > > Cc: Bob Feng <bob.c.feng@intel.com>
> > > ---
> > > In V2:
> > >   Fix GCC8 compiler issue.
> > >
> > > In V3:
> > >   Fix snprintf wrong replacement.
> > >
> > >  BaseTools/Source/C/BfmLib/BfmLib.c                 |
> 117 +++++++++++----------
> > >  BaseTools/Source/C/FCE/BinaryParse.c               |
> 2 +-
> > >  BaseTools/Source/C/FMMT/FirmwareModuleManagement.c |
> 2 +-
> > >  BaseTools/Source/C/FMMT/FmmtLib.c                  |
> 117 +++++++++++----------
> > >  4 files changed, 122 insertions(+), 116 deletions(-)
> > >
> > > diff --git a/BaseTools/Source/C/BfmLib/BfmLib.c
> > > b/BaseTools/Source/C/BfmLib/BfmLib.c
> > > index 9dedda3da2..8d0ec9038c 100644
> > > --- a/BaseTools/Source/C/BfmLib/BfmLib.c
> > > +++ b/BaseTools/Source/C/BfmLib/BfmLib.c
> > > @@ -9,6 +9,9 @@
> > >
> > >  #include "BinFileManager.h"
> > >
> > > +#define STR_LEN_MAX_4K 4096
> > > +#define STR_LEN_MAX_1K 1024
> > > +
> >
> > Coding style 3.3.3: "Code files should not contain
> #define and typedef
> > statements."
> >
> > Move these to BinFileManager.h?
> >
> > >  #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes,
> TestAttributes, Bit) \
> > >      ( \
> > >        (BOOLEAN) ( \
> > > @@ -116,7 +119,7 @@ LibInitializeFvStruct (
> > >
> > >    for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV;
> Index ++) {
> > >      memset (Fv->FfsAttuibutes[Index].FfsName, '\0',
> _MAX_PATH);
> > > -    memset (Fv->FfsAttuibutes[Index].UiName, '\0',
> _MAX_PATH);
> > > +    memset (Fv->FfsAttuibutes[Index].UiName, '\0',
> _MAX_PATH *
> > > + sizeof (CHAR16));
> > >
> > >      Fv->FfsAttuibutes[Index].IsLeaf               =
> TRUE;
> > >      Fv->FfsAttuibutes[Index].TotalSectionNum      =
> 0;
> > > @@ -2399,153 +2402,153 @@ LibFvHeaderAttributeToStr (
> > >
> > >    LocalStr  = NULL;
> > >
> > > -  LocalStr = (CHAR8 *) malloc (1024 * 4);
> > > +  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
> > >
> > >    if (LocalStr == NULL) {
> > >      printf ("Out of resource, memory allocation
> failed. \n");
> > >      return EFI_OUT_OF_RESOURCES;
> > >    }
> > >
> > > -  memset (LocalStr, '\0', 1024 * 4);
> > > +  memset (LocalStr, '\0', STR_LEN_MAX_4K);
> > >
> > >    if (Attr == 0 || InfFile  == NULL) {
> > >      free (LocalStr);
> > >      return EFI_INVALID_PARAMETER;
> > >    }
> > >
> > > -  strncat (LocalStr, "[attributes] \n",
> sizeof("[attributes] \n"));
> > > +  strncat (LocalStr, "[attributes] \n",
> STR_LEN_MAX_4K - strlen
> > > + (LocalStr) - 1);
> >
> > This is a very inefficient, and difficult to read, way
> of doing this.
> > It also performs absolutely no error checking (making
> any future
> > debugging tedious at best).
> >
> This change is to fix gcc warning -Werror=sizeof-pointer-
> memaccess.
> strncat is the safe version API. So, it is used here.
> 
> > I have to be honest - looking at this code, I think the
> right thing to
> > do would be to revert the commit adding it and
> reworking the utility
> > completely (making sure it gets serious review) before
> merging it.
> >
> > I am not surprised the compilers get upset.
> >
> > This made me have a closer look at the patches adding
> FCE and FMMT,
> > and frankly, my opinion of them are similar. These
> aren't being
> > upstreamed - this is textbook "chuck over the wall".
> >
> > Don't get me wrong - the code quality of FMMT is
> notably higher than
> > the code quality of FCE, which is notably higher than
> the code quality
> > of BfmLib. But even
> >
> > BfmLib was added with the comment that it is used by
> FCE.
> > So why do FCE and FMMT both have separate copies of
> LibBfmGuidToStr?
> > Not just functions doing the same thing, but with the
> same name.
> >
> > There are many other issues with these tools.
> 
> FCE & FMMT both operate the binary FV image. Their
> implementation may copy the same logic. I don't think
> this is a good way.
> I agree to do better design for code sharing and code
> quality improvement.
> 
> I suggest to revert them, add current FCE & FMMT into
> edk2-staging, then refine them, and send the patch to add
> them back into edk2 BaseTools.
> 
> Thanks
> Liming
> >
> > /
> >     Leif
> >
> > >
> > >    if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
> > > -    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE
> \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
> > > -    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE
> \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_READ_STATUS) {
> > > -    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n",
> sizeof ("EFI_READ_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
> > > -    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP =
> TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
> > > -    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE
> \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_STATUS) {
> > > -    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n",
> sizeof ("EFI_WRITE_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_LOCK_CAP) {
> > > -    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n",
> sizeof ("EFI_LOCK_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n",
> STR_LEN_MAX_4K -
> > > + strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_LOCK_STATUS) {
> > > -    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n",
> sizeof ("EFI_LOCK_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_STICKY_WRITE) {
> > > -    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n",
> sizeof ("EFI_STICKY_WRITE = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_MEMORY_MAPPED) {
> > > -    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE
> \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_ERASE_POLARITY) {
> > > -    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n",
> sizeof ("EFI_ERASE_POLARITY = 1 \n"));
> > > +    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_READ_LOCK_CAP) {
> > > -    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE
> \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
> > > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
> \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
> > > -    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE
> \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
> > > -    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE
> \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_LOCK_STATUS) {
> > > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
> \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    //
> > >    // Alignment
> > >    //
> > >    if (Attr & EFI_FVB2_ALIGNMENT_1) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (fwrite (LocalStr, 1, (size_t) strlen
> (LocalStr), InfFile) !=
> > > (size_t) strlen (LocalStr)) { @@ -2592,7 +2595,7 @@
> LibFvHeaderOptionToStr (
> > >    //
> > >    // This section will not over 1024 bytes and each
> line will never over 128 bytes.
> > >    //
> > > -  LocalStr    = (CHAR8 *) malloc (1024);
> > > +  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
> > >    BlockSize   = (CHAR8 *) malloc (128);
> > >    NumOfBlocks = (CHAR8 *) malloc (128);
> > >
> > > @@ -2611,18 +2614,18 @@ LibFvHeaderOptionToStr (
> > >      return EFI_OUT_OF_RESOURCES;
> > >    }
> > >
> > > -  memset (LocalStr, '\0', 1024);
> > > +  memset (LocalStr, '\0', STR_LEN_MAX_1K);
> > >    memset (BlockSize, '\0', 128);
> > >    memset (NumOfBlocks, '\0', 128);
> > >
> > > -  strncat (LocalStr, "[options] \n",
> sizeof("[Options] \n"));
> > > +  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K
> - strlen
> > > + (LocalStr) - 1);
> > >
> > >    sprintf (BlockSize, "EFI_BLOCK_SIZE  = 0x%x \n",
> > > BlockMap->Length);
> > > -  strncat (LocalStr, BlockSize, strlen(BlockSize));
> > > +  strncat (LocalStr, BlockSize, STR_LEN_MAX_1K -
> strlen (LocalStr)
> > > + - 1);
> > >
> > >    if (IsRootFv) {
> > >    sprintf (NumOfBlocks, "EFI_NUM_BLOCKS  = 0x%x \n",
> > > BlockMap->NumBlocks);
> > > -  strncat (LocalStr, NumOfBlocks,
> strlen(NumOfBlocks));
> > > +  strncat (LocalStr, NumOfBlocks, STR_LEN_MAX_1K -
> strlen
> > > + (LocalStr) - 1);
> > >    }
> > >
> > >    if (fwrite (LocalStr, 1, (size_t) strlen
> (LocalStr), InfFile) !=
> > > (size_t) strlen (LocalStr)) { diff --git
> > > a/BaseTools/Source/C/FCE/BinaryParse.c
> > > b/BaseTools/Source/C/FCE/BinaryParse.c
> > > index e9f8ee6826..97d6ecf93b 100644
> > > --- a/BaseTools/Source/C/FCE/BinaryParse.c
> > > +++ b/BaseTools/Source/C/FCE/BinaryParse.c
> > > @@ -1240,7 +1240,7 @@ Done:
> > >        ) {
> > >          continue;
> > >        }
> > > -      sprintf (FileNameArry, "%s%c%s", FolderName,
> OS_SEP, pDirent->d_name);
> > > +      snprintf (FileNameArry, MAX_FILENAME_LEN,
> "%s%c%s",
> > > + FolderName, OS_SEP, pDirent->d_name);
> > >        FfsFile = fopen (FileNameArry, "rb");
> > >        Status = ReadFfsHeader (FfsFile, (UINT32
> *)&FileSize);
> > >        if (EFI_ERROR (Status)) {
> > > diff --git
> a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> > > b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> > > index 63ae3c45a4..6648fbd54f 100644
> > > ---
> a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> > > +++
> b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
> > > @@ -1572,7 +1572,7 @@ FmmtImageDelete (
> > >                      // If decrease operation
> executed, we should adjust the ffs list. It will bring in
> more complex.
> > >                      //
> > >                      //FvInFd->FfsNumbers
> -= 1;
> > > -                    memset(FvInFd-
> >FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
> > > +                    memset(FvInFd-
> >FfsAttuibutes[Index].UiName,
> > > + '\0', _MAX_PATH * sizeof (CHAR16));
> > >                     if (FvInFd-
> >FfsAttuibutes[Index].FvLevel > 1) {
> > >                         for (j = Index - 1; j >= 0;
> j--) {
> > >                             if (FvInFd-
> >FfsAttuibutes[j].FvLevel ==
> > > FvInFd->FfsAttuibutes[Index].FvLevel - 1) { diff --
> git
> > > a/BaseTools/Source/C/FMMT/FmmtLib.c
> > > b/BaseTools/Source/C/FMMT/FmmtLib.c
> > > index f87042114b..e236810c59 100644
> > > --- a/BaseTools/Source/C/FMMT/FmmtLib.c
> > > +++ b/BaseTools/Source/C/FMMT/FmmtLib.c
> > > @@ -9,6 +9,9 @@
> > >
> > >  #include "FirmwareModuleManagement.h"
> > >
> > > +#define STR_LEN_MAX_4K 4096
> > > +#define STR_LEN_MAX_1K 1024
> > > +
> > >  #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes,
> TestAttributes, Bit) \
> > >      ( \
> > >        (BOOLEAN) ( \
> > > @@ -155,7 +158,7 @@ LibInitializeFvStruct (
> > >
> > >    for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV;
> Index ++) {
> > >      memset (Fv->FfsAttuibutes[Index].FfsName, '\0',
> _MAX_PATH);
> > > -    memset (Fv->FfsAttuibutes[Index].UiName, '\0',
> _MAX_PATH);
> > > +    memset (Fv->FfsAttuibutes[Index].UiName, '\0',
> _MAX_PATH *
> > > + sizeof (CHAR16));
> > >      memset (&Fv->FfsAttuibutes[Index].GuidName,
> '\0', sizeof(EFI_GUID));
> > >      Fv->FfsAttuibutes[Index].UiNameSize           =
> 0;
> > >      Fv->FfsAttuibutes[Index].IsLeaf               =
> TRUE;
> > > @@ -2504,153 +2507,153 @@ LibFvHeaderAttributeToStr (
> > >
> > >    LocalStr  = NULL;
> > >
> > > -  LocalStr = (CHAR8 *) malloc (1024 * 4);
> > > +  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
> > >
> > >    if (LocalStr == NULL) {
> > >      printf ("Memory allocate error!\n");
> > >      return EFI_OUT_OF_RESOURCES;
> > >    }
> > >
> > > -  memset (LocalStr, '\0', 1024 * 4);
> > > +  memset (LocalStr, '\0', STR_LEN_MAX_4K);
> > >
> > >    if (Attr == 0 || InfFile  == NULL) {
> > >      free (LocalStr);
> > >      return EFI_INVALID_PARAMETER;
> > >    }
> > >
> > > -  strncat (LocalStr, "[attributes] \n",
> sizeof("[attributes] \n"));
> > > +  strncat (LocalStr, "[attributes] \n",
> STR_LEN_MAX_4K - strlen
> > > + (LocalStr) - 1);
> > >
> > >    if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
> > > -    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE
> \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
> > > -    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE
> \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_READ_STATUS) {
> > > -    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n",
> sizeof ("EFI_READ_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
> > > -    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP =
> TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
> > > -    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE
> \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_STATUS) {
> > > -    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n",
> sizeof ("EFI_WRITE_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_LOCK_CAP) {
> > > -    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n",
> sizeof ("EFI_LOCK_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n",
> STR_LEN_MAX_4K -
> > > + strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_LOCK_STATUS) {
> > > -    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n",
> sizeof ("EFI_LOCK_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_STICKY_WRITE) {
> > > -    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n",
> sizeof ("EFI_STICKY_WRITE = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_MEMORY_MAPPED) {
> > > -    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE
> \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_ERASE_POLARITY) {
> > > -    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n",
> sizeof ("EFI_ERASE_POLARITY = 1 \n"));
> > > +    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n",
> STR_LEN_MAX_4K
> > > + - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_READ_LOCK_CAP) {
> > > -    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE
> \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
> > > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
> \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
> > > -    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE
> \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
> > > -    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE
> \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (Attr & EFI_FVB2_LOCK_STATUS) {
> > > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
> \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    //
> > >    // Alignment
> > >    //
> > >    if (Attr & EFI_FVB2_ALIGNMENT_1) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M =
> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M =
> TRUE \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE
> \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE
> \n",
> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
> > >    }
> > >
> > >    if (fwrite (LocalStr, 1, (size_t) strlen
> (LocalStr), InfFile) !=
> > > (size_t) strlen (LocalStr)) { @@ -2696,7 +2699,7 @@
> LibFvHeaderOptionToStr (
> > >    //
> > >    // This section will not over 1024 bytes and each
> line will never over 128 bytes.
> > >    //
> > > -  LocalStr    = (CHAR8 *) malloc (1024);
> > > +  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
> > >    TempStr     = (CHAR8 *) malloc (128);
> > >
> > >    if (LocalStr    == NULL ||
> > > @@ -2712,18 +2715,18 @@ LibFvHeaderOptionToStr (
> > >    }
> > >
> > >    BlockMap = FvHeader->BlockMap;
> > > -  memset (LocalStr, '\0', 1024);
> > > +  memset (LocalStr, '\0', STR_LEN_MAX_1K);
> > >    memset (TempStr, '\0', 128);
> > >
> > > -  strncat (LocalStr, "[options] \n",
> sizeof("[Options] \n"));
> > > +  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K
> - strlen
> > > + (LocalStr) - 1);
> > >
> > >
> > >    snprintf (TempStr, 128, "EFI_BLOCK_SIZE  = 0x%x
> \n",
> > > BlockMap->Length);
> > > -  strncat (LocalStr, TempStr, strlen(TempStr));
> > > +  strncat (LocalStr, TempStr, STR_LEN_MAX_1K -
> strlen (LocalStr) -
> > > + 1);
> > >
> > >    if (IsRootFv) {
> > >    snprintf (TempStr, 128, "EFI_NUM_BLOCKS  = 0x%x
> \n",
> > > BlockMap->NumBlocks);
> > > -  strncat (LocalStr, TempStr, strlen(TempStr));
> > > +  strncat (LocalStr, TempStr, STR_LEN_MAX_1K -
> strlen (LocalStr) -
> > > + 1);
> > >    }
> > >
> > >    if (fwrite (LocalStr, 1, (size_t) strlen
> (LocalStr), InfFile) !=
> > > (size_t) strlen (LocalStr)) {
> > > --
> > > 2.13.0.windows.1
> > >
> > >
> > >
> > >
> >
> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43526): https://edk2.groups.io/g/devel/message/43526
Mute This Topic: https://groups.io/mt/32403705/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.

Posted by Liming Gao 18 weeks ago
Mike:
  Yes. I send the patch and revert them today. 

Thanks
Liming
>-----Original Message-----
>From: Kinney, Michael D
>Sent: Thursday, July 11, 2019 4:10 AM
>To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io;
>leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>
>Cc: Gao, Zhichao <zhichao.gao@intel.com>; Feng, Bob C
><bob.c.feng@intel.com>; Andrew Fish <afish@apple.com>; Laszlo Ersek
><lersek@redhat.com>
>Subject: RE: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in
>new added tools.
>
>Liming,
>
>I agree that edk2-staging is a good place to work on cleaning
>these tools up.
>
>Please revert the commit of these tools today so GCC builds
>are not broken.
>
>Thanks,
>
>Mike
>
>> -----Original Message-----
>> From: Gao, Liming
>> Sent: Wednesday, July 10, 2019 6:43 AM
>> To: devel@edk2.groups.io; leif.lindholm@linaro.org
>> Cc: Gao, Zhichao <zhichao.gao@intel.com>; Feng, Bob C
>> <bob.c.feng@intel.com>; Andrew Fish <afish@apple.com>;
>> Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Subject: RE: [edk2-devel] [Patch v3] BaseTools: Fix GCC
>> compiler failure in new added tools.
>>
>> Lefi:
>>
>> > -----Original Message-----
>> > From: devel@edk2.groups.io
>> [mailto:devel@edk2.groups.io] On Behalf Of
>> > Leif Lindholm
>> > Sent: Wednesday, July 10, 2019 2:26 AM
>> > To: devel@edk2.groups.io; Gao, Liming
>> <liming.gao@intel.com>
>> > Cc: Gao, Zhichao <zhichao.gao@intel.com>; Feng, Bob C
>> > <bob.c.feng@intel.com>; Andrew Fish <afish@apple.com>;
>> Laszlo Ersek
>> > <lersek@redhat.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> > Subject: Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC
>> compiler failure in new added tools.
>> >
>> > Hi Liming,
>> >
>> > On Tue, Jul 09, 2019 at 05:53:33PM +0800, Liming Gao
>> wrote:
>> > > From: gaozhic <zhichao.gao@intel.com>
>> > >
>> > > GCC 7 or 8 reports some warnings in new added
>> FCE/FMMT/BlmLib.
>> >
>> > Please list the specific warnings addressed.
>> > (But also see my comments below - I think this utility
>> needs ripping
>> > out and rewriting.)
>> >
>> > > Signed-off-by: Liming Gao <liming.gao@intel.com>
>> > > Cc: Bob Feng <bob.c.feng@intel.com>
>> > > ---
>> > > In V2:
>> > >   Fix GCC8 compiler issue.
>> > >
>> > > In V3:
>> > >   Fix snprintf wrong replacement.
>> > >
>> > >  BaseTools/Source/C/BfmLib/BfmLib.c                 |
>> 117 +++++++++++----------
>> > >  BaseTools/Source/C/FCE/BinaryParse.c               |
>> 2 +-
>> > >  BaseTools/Source/C/FMMT/FirmwareModuleManagement.c |
>> 2 +-
>> > >  BaseTools/Source/C/FMMT/FmmtLib.c                  |
>> 117 +++++++++++----------
>> > >  4 files changed, 122 insertions(+), 116 deletions(-)
>> > >
>> > > diff --git a/BaseTools/Source/C/BfmLib/BfmLib.c
>> > > b/BaseTools/Source/C/BfmLib/BfmLib.c
>> > > index 9dedda3da2..8d0ec9038c 100644
>> > > --- a/BaseTools/Source/C/BfmLib/BfmLib.c
>> > > +++ b/BaseTools/Source/C/BfmLib/BfmLib.c
>> > > @@ -9,6 +9,9 @@
>> > >
>> > >  #include "BinFileManager.h"
>> > >
>> > > +#define STR_LEN_MAX_4K 4096
>> > > +#define STR_LEN_MAX_1K 1024
>> > > +
>> >
>> > Coding style 3.3.3: "Code files should not contain
>> #define and typedef
>> > statements."
>> >
>> > Move these to BinFileManager.h?
>> >
>> > >  #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes,
>> TestAttributes, Bit) \
>> > >      ( \
>> > >        (BOOLEAN) ( \
>> > > @@ -116,7 +119,7 @@ LibInitializeFvStruct (
>> > >
>> > >    for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV;
>> Index ++) {
>> > >      memset (Fv->FfsAttuibutes[Index].FfsName, '\0',
>> _MAX_PATH);
>> > > -    memset (Fv->FfsAttuibutes[Index].UiName, '\0',
>> _MAX_PATH);
>> > > +    memset (Fv->FfsAttuibutes[Index].UiName, '\0',
>> _MAX_PATH *
>> > > + sizeof (CHAR16));
>> > >
>> > >      Fv->FfsAttuibutes[Index].IsLeaf               =
>> TRUE;
>> > >      Fv->FfsAttuibutes[Index].TotalSectionNum      =
>> 0;
>> > > @@ -2399,153 +2402,153 @@ LibFvHeaderAttributeToStr (
>> > >
>> > >    LocalStr  = NULL;
>> > >
>> > > -  LocalStr = (CHAR8 *) malloc (1024 * 4);
>> > > +  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
>> > >
>> > >    if (LocalStr == NULL) {
>> > >      printf ("Out of resource, memory allocation
>> failed. \n");
>> > >      return EFI_OUT_OF_RESOURCES;
>> > >    }
>> > >
>> > > -  memset (LocalStr, '\0', 1024 * 4);
>> > > +  memset (LocalStr, '\0', STR_LEN_MAX_4K);
>> > >
>> > >    if (Attr == 0 || InfFile  == NULL) {
>> > >      free (LocalStr);
>> > >      return EFI_INVALID_PARAMETER;
>> > >    }
>> > >
>> > > -  strncat (LocalStr, "[attributes] \n",
>> sizeof("[attributes] \n"));
>> > > +  strncat (LocalStr, "[attributes] \n",
>> STR_LEN_MAX_4K - strlen
>> > > + (LocalStr) - 1);
>> >
>> > This is a very inefficient, and difficult to read, way
>> of doing this.
>> > It also performs absolutely no error checking (making
>> any future
>> > debugging tedious at best).
>> >
>> This change is to fix gcc warning -Werror=sizeof-pointer-
>> memaccess.
>> strncat is the safe version API. So, it is used here.
>>
>> > I have to be honest - looking at this code, I think the
>> right thing to
>> > do would be to revert the commit adding it and
>> reworking the utility
>> > completely (making sure it gets serious review) before
>> merging it.
>> >
>> > I am not surprised the compilers get upset.
>> >
>> > This made me have a closer look at the patches adding
>> FCE and FMMT,
>> > and frankly, my opinion of them are similar. These
>> aren't being
>> > upstreamed - this is textbook "chuck over the wall".
>> >
>> > Don't get me wrong - the code quality of FMMT is
>> notably higher than
>> > the code quality of FCE, which is notably higher than
>> the code quality
>> > of BfmLib. But even
>> >
>> > BfmLib was added with the comment that it is used by
>> FCE.
>> > So why do FCE and FMMT both have separate copies of
>> LibBfmGuidToStr?
>> > Not just functions doing the same thing, but with the
>> same name.
>> >
>> > There are many other issues with these tools.
>>
>> FCE & FMMT both operate the binary FV image. Their
>> implementation may copy the same logic. I don't think
>> this is a good way.
>> I agree to do better design for code sharing and code
>> quality improvement.
>>
>> I suggest to revert them, add current FCE & FMMT into
>> edk2-staging, then refine them, and send the patch to add
>> them back into edk2 BaseTools.
>>
>> Thanks
>> Liming
>> >
>> > /
>> >     Leif
>> >
>> > >
>> > >    if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
>> > > -    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE
>> \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
>> > > -    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE
>> \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_READ_STATUS) {
>> > > -    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n",
>> sizeof ("EFI_READ_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
>> > > -    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP =
>> TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
>> > > -    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE
>> \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_STATUS) {
>> > > -    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n",
>> sizeof ("EFI_WRITE_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_LOCK_CAP) {
>> > > -    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n",
>> sizeof ("EFI_LOCK_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n",
>> STR_LEN_MAX_4K -
>> > > + strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_LOCK_STATUS) {
>> > > -    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n",
>> sizeof ("EFI_LOCK_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_STICKY_WRITE) {
>> > > -    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n",
>> sizeof ("EFI_STICKY_WRITE = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_MEMORY_MAPPED) {
>> > > -    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE
>> \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_ERASE_POLARITY) {
>> > > -    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n",
>> sizeof ("EFI_ERASE_POLARITY = 1 \n"));
>> > > +    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_READ_LOCK_CAP) {
>> > > -    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE
>> \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
>> > > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
>> \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
>> > > -    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE
>> \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
>> > > -    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE
>> \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_LOCK_STATUS) {
>> > > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
>> \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    //
>> > >    // Alignment
>> > >    //
>> > >    if (Attr & EFI_FVB2_ALIGNMENT_1) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (fwrite (LocalStr, 1, (size_t) strlen
>> (LocalStr), InfFile) !=
>> > > (size_t) strlen (LocalStr)) { @@ -2592,7 +2595,7 @@
>> LibFvHeaderOptionToStr (
>> > >    //
>> > >    // This section will not over 1024 bytes and each
>> line will never over 128 bytes.
>> > >    //
>> > > -  LocalStr    = (CHAR8 *) malloc (1024);
>> > > +  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
>> > >    BlockSize   = (CHAR8 *) malloc (128);
>> > >    NumOfBlocks = (CHAR8 *) malloc (128);
>> > >
>> > > @@ -2611,18 +2614,18 @@ LibFvHeaderOptionToStr (
>> > >      return EFI_OUT_OF_RESOURCES;
>> > >    }
>> > >
>> > > -  memset (LocalStr, '\0', 1024);
>> > > +  memset (LocalStr, '\0', STR_LEN_MAX_1K);
>> > >    memset (BlockSize, '\0', 128);
>> > >    memset (NumOfBlocks, '\0', 128);
>> > >
>> > > -  strncat (LocalStr, "[options] \n",
>> sizeof("[Options] \n"));
>> > > +  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K
>> - strlen
>> > > + (LocalStr) - 1);
>> > >
>> > >    sprintf (BlockSize, "EFI_BLOCK_SIZE  = 0x%x \n",
>> > > BlockMap->Length);
>> > > -  strncat (LocalStr, BlockSize, strlen(BlockSize));
>> > > +  strncat (LocalStr, BlockSize, STR_LEN_MAX_1K -
>> strlen (LocalStr)
>> > > + - 1);
>> > >
>> > >    if (IsRootFv) {
>> > >    sprintf (NumOfBlocks, "EFI_NUM_BLOCKS  = 0x%x \n",
>> > > BlockMap->NumBlocks);
>> > > -  strncat (LocalStr, NumOfBlocks,
>> strlen(NumOfBlocks));
>> > > +  strncat (LocalStr, NumOfBlocks, STR_LEN_MAX_1K -
>> strlen
>> > > + (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (fwrite (LocalStr, 1, (size_t) strlen
>> (LocalStr), InfFile) !=
>> > > (size_t) strlen (LocalStr)) { diff --git
>> > > a/BaseTools/Source/C/FCE/BinaryParse.c
>> > > b/BaseTools/Source/C/FCE/BinaryParse.c
>> > > index e9f8ee6826..97d6ecf93b 100644
>> > > --- a/BaseTools/Source/C/FCE/BinaryParse.c
>> > > +++ b/BaseTools/Source/C/FCE/BinaryParse.c
>> > > @@ -1240,7 +1240,7 @@ Done:
>> > >        ) {
>> > >          continue;
>> > >        }
>> > > -      sprintf (FileNameArry, "%s%c%s", FolderName,
>> OS_SEP, pDirent->d_name);
>> > > +      snprintf (FileNameArry, MAX_FILENAME_LEN,
>> "%s%c%s",
>> > > + FolderName, OS_SEP, pDirent->d_name);
>> > >        FfsFile = fopen (FileNameArry, "rb");
>> > >        Status = ReadFfsHeader (FfsFile, (UINT32
>> *)&FileSize);
>> > >        if (EFI_ERROR (Status)) {
>> > > diff --git
>> a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
>> > > b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
>> > > index 63ae3c45a4..6648fbd54f 100644
>> > > ---
>> a/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
>> > > +++
>> b/BaseTools/Source/C/FMMT/FirmwareModuleManagement.c
>> > > @@ -1572,7 +1572,7 @@ FmmtImageDelete (
>> > >                      // If decrease operation
>> executed, we should adjust the ffs list. It will bring in
>> more complex.
>> > >                      //
>> > >                      //FvInFd->FfsNumbers
>> -= 1;
>> > > -                    memset(FvInFd-
>> >FfsAttuibutes[Index].UiName, '\0', _MAX_PATH);
>> > > +                    memset(FvInFd-
>> >FfsAttuibutes[Index].UiName,
>> > > + '\0', _MAX_PATH * sizeof (CHAR16));
>> > >                     if (FvInFd-
>> >FfsAttuibutes[Index].FvLevel > 1) {
>> > >                         for (j = Index - 1; j >= 0;
>> j--) {
>> > >                             if (FvInFd-
>> >FfsAttuibutes[j].FvLevel ==
>> > > FvInFd->FfsAttuibutes[Index].FvLevel - 1) { diff --
>> git
>> > > a/BaseTools/Source/C/FMMT/FmmtLib.c
>> > > b/BaseTools/Source/C/FMMT/FmmtLib.c
>> > > index f87042114b..e236810c59 100644
>> > > --- a/BaseTools/Source/C/FMMT/FmmtLib.c
>> > > +++ b/BaseTools/Source/C/FMMT/FmmtLib.c
>> > > @@ -9,6 +9,9 @@
>> > >
>> > >  #include "FirmwareModuleManagement.h"
>> > >
>> > > +#define STR_LEN_MAX_4K 4096
>> > > +#define STR_LEN_MAX_1K 1024
>> > > +
>> > >  #define EFI_TEST_FFS_ATTRIBUTES_BIT(FvbAttributes,
>> TestAttributes, Bit) \
>> > >      ( \
>> > >        (BOOLEAN) ( \
>> > > @@ -155,7 +158,7 @@ LibInitializeFvStruct (
>> > >
>> > >    for (Index = 0; Index < MAX_NUMBER_OF_FILES_IN_FV;
>> Index ++) {
>> > >      memset (Fv->FfsAttuibutes[Index].FfsName, '\0',
>> _MAX_PATH);
>> > > -    memset (Fv->FfsAttuibutes[Index].UiName, '\0',
>> _MAX_PATH);
>> > > +    memset (Fv->FfsAttuibutes[Index].UiName, '\0',
>> _MAX_PATH *
>> > > + sizeof (CHAR16));
>> > >      memset (&Fv->FfsAttuibutes[Index].GuidName,
>> '\0', sizeof(EFI_GUID));
>> > >      Fv->FfsAttuibutes[Index].UiNameSize           =
>> 0;
>> > >      Fv->FfsAttuibutes[Index].IsLeaf               =
>> TRUE;
>> > > @@ -2504,153 +2507,153 @@ LibFvHeaderAttributeToStr (
>> > >
>> > >    LocalStr  = NULL;
>> > >
>> > > -  LocalStr = (CHAR8 *) malloc (1024 * 4);
>> > > +  LocalStr = (CHAR8 *) malloc (STR_LEN_MAX_4K);
>> > >
>> > >    if (LocalStr == NULL) {
>> > >      printf ("Memory allocate error!\n");
>> > >      return EFI_OUT_OF_RESOURCES;
>> > >    }
>> > >
>> > > -  memset (LocalStr, '\0', 1024 * 4);
>> > > +  memset (LocalStr, '\0', STR_LEN_MAX_4K);
>> > >
>> > >    if (Attr == 0 || InfFile  == NULL) {
>> > >      free (LocalStr);
>> > >      return EFI_INVALID_PARAMETER;
>> > >    }
>> > >
>> > > -  strncat (LocalStr, "[attributes] \n",
>> sizeof("[attributes] \n"));
>> > > +  strncat (LocalStr, "[attributes] \n",
>> STR_LEN_MAX_4K - strlen
>> > > + (LocalStr) - 1);
>> > >
>> > >    if (Attr & EFI_FVB2_READ_DISABLED_CAP) {
>> > > -    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE
>> \n", sizeof ("EFI_READ_DISABLED_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_DISABLED_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_READ_ENABLED_CAP) {
>> > > -    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE
>> \n", sizeof ("EFI_READ_ENABLED_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_ENABLED_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_READ_STATUS) {
>> > > -    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n",
>> sizeof ("EFI_READ_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_STATUS = TRUE \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_DISABLED_CAP) {
>> > > -    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP =
>> TRUE \n", sizeof ("EFI_WRITE_DISABLED_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_DISABLED_CAP =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_ENABLED_CAP) {
>> > > -    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE
>> \n", sizeof ("EFI_WRITE_ENABLED_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_ENABLED_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_STATUS) {
>> > > -    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n",
>> sizeof ("EFI_WRITE_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_STATUS = TRUE \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_LOCK_CAP) {
>> > > -    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n",
>> sizeof ("EFI_LOCK_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_LOCK_CAP = TRUE \n",
>> STR_LEN_MAX_4K -
>> > > + strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_LOCK_STATUS) {
>> > > -    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n",
>> sizeof ("EFI_LOCK_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_LOCK_STATUS = TRUE \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_STICKY_WRITE) {
>> > > -    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n",
>> sizeof ("EFI_STICKY_WRITE = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_STICKY_WRITE = TRUE \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_MEMORY_MAPPED) {
>> > > -    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE
>> \n", sizeof ("EFI_MEMORY_MAPPED = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_MEMORY_MAPPED = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_ERASE_POLARITY) {
>> > > -    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n",
>> sizeof ("EFI_ERASE_POLARITY = 1 \n"));
>> > > +    strncat (LocalStr, "EFI_ERASE_POLARITY = 1 \n",
>> STR_LEN_MAX_4K
>> > > + - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_READ_LOCK_CAP) {
>> > > -    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE
>> \n", sizeof ("EFI_READ_LOCK_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_LOCK_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_READ_LOCK_STATUS) {
>> > > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
>> \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_LOCK_CAP) {
>> > > -    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE
>> \n", sizeof ("EFI_WRITE_LOCK_CAP = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_LOCK_CAP = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_WRITE_LOCK_STATUS) {
>> > > -    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE
>> \n", sizeof ("EFI_WRITE_LOCK_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_WRITE_LOCK_STATUS = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (Attr & EFI_FVB2_LOCK_STATUS) {
>> > > -    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
>> \n", sizeof ("EFI_READ_LOCK_STATUS = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_READ_LOCK_STATUS = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    //
>> > >    // Alignment
>> > >    //
>> > >    if (Attr & EFI_FVB2_ALIGNMENT_1) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_1 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_2 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_4 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_8 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_16 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_32 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_64 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64 = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128 =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256 =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512 = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512 =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_1K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1K = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_2K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2K = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_4K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4K = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_8K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8K = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512K) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512K = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512K =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_1M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1M = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_2M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2M = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_4M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_4M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_4M = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_8M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_8M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_8M = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_16M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_16M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_16M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_32M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_32M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_32M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_64M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_64M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_64M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_128M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_128M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_128M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_256M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_256M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_256M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_512M) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M =
>> TRUE \n", sizeof ("EFI_FVB2_ALIGNMENT_512M = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_512M =
>> TRUE \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_1G) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_1G = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_1G = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    } else if (Attr & EFI_FVB2_ALIGNMENT_2G) {
>> > > -    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE
>> \n", sizeof ("EFI_FVB2_ALIGNMENT_2G = TRUE \n"));
>> > > +    strncat (LocalStr, "EFI_FVB2_ALIGNMENT_2G = TRUE
>> \n",
>> > > + STR_LEN_MAX_4K - strlen (LocalStr) - 1);
>> > >    }
>> > >
>> > >    if (fwrite (LocalStr, 1, (size_t) strlen
>> (LocalStr), InfFile) !=
>> > > (size_t) strlen (LocalStr)) { @@ -2696,7 +2699,7 @@
>> LibFvHeaderOptionToStr (
>> > >    //
>> > >    // This section will not over 1024 bytes and each
>> line will never over 128 bytes.
>> > >    //
>> > > -  LocalStr    = (CHAR8 *) malloc (1024);
>> > > +  LocalStr    = (CHAR8 *) malloc (STR_LEN_MAX_1K);
>> > >    TempStr     = (CHAR8 *) malloc (128);
>> > >
>> > >    if (LocalStr    == NULL ||
>> > > @@ -2712,18 +2715,18 @@ LibFvHeaderOptionToStr (
>> > >    }
>> > >
>> > >    BlockMap = FvHeader->BlockMap;
>> > > -  memset (LocalStr, '\0', 1024);
>> > > +  memset (LocalStr, '\0', STR_LEN_MAX_1K);
>> > >    memset (TempStr, '\0', 128);
>> > >
>> > > -  strncat (LocalStr, "[options] \n",
>> sizeof("[Options] \n"));
>> > > +  strncat (LocalStr, "[options] \n", STR_LEN_MAX_1K
>> - strlen
>> > > + (LocalStr) - 1);
>> > >
>> > >
>> > >    snprintf (TempStr, 128, "EFI_BLOCK_SIZE  = 0x%x
>> \n",
>> > > BlockMap->Length);
>> > > -  strncat (LocalStr, TempStr, strlen(TempStr));
>> > > +  strncat (LocalStr, TempStr, STR_LEN_MAX_1K -
>> strlen (LocalStr) -
>> > > + 1);
>> > >
>> > >    if (IsRootFv) {
>> > >    snprintf (TempStr, 128, "EFI_NUM_BLOCKS  = 0x%x
>> \n",
>> > > BlockMap->NumBlocks);
>> > > -  strncat (LocalStr, TempStr, strlen(TempStr));
>> > > +  strncat (LocalStr, TempStr, STR_LEN_MAX_1K -
>> strlen (LocalStr) -
>> > > + 1);
>> > >    }
>> > >
>> > >    if (fwrite (LocalStr, 1, (size_t) strlen
>> (LocalStr), InfFile) !=
>> > > (size_t) strlen (LocalStr)) {
>> > > --
>> > > 2.13.0.windows.1
>> > >
>> > >
>> > >
>> > >
>> >
>> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43544): https://edk2.groups.io/g/devel/message/43544
Mute This Topic: https://groups.io/mt/32403705/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.

Posted by Gary Lin 18 weeks ago
On Tue, Jul 09, 2019 at 05:53:33PM +0800,  Liming Gao  wrote:
> From: gaozhic <zhichao.gao@intel.com>
> 
> GCC 7 or 8 reports some warnings in new added FCE/FMMT/BlmLib.
> 
> Signed-off-by: Liming Gao <liming.gao@intel.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> ---
> In V2: 
>   Fix GCC8 compiler issue.
> 
> In V3:
>   Fix snprintf wrong replacement.
> 
>  BaseTools/Source/C/BfmLib/BfmLib.c                 | 117 +++++++++++----------
>  BaseTools/Source/C/FCE/BinaryParse.c               |   2 +-
>  BaseTools/Source/C/FMMT/FirmwareModuleManagement.c |   2 +-
>  BaseTools/Source/C/FMMT/FmmtLib.c                  | 117 +++++++++++----------
>  4 files changed, 122 insertions(+), 116 deletions(-)
> 
-->8--

> diff --git a/BaseTools/Source/C/FCE/BinaryParse.c b/BaseTools/Source/C/FCE/BinaryParse.c
> index e9f8ee6826..97d6ecf93b 100644
> --- a/BaseTools/Source/C/FCE/BinaryParse.c
> +++ b/BaseTools/Source/C/FCE/BinaryParse.c
> @@ -1240,7 +1240,7 @@ Done:
>        ) {
>          continue;
>        }
> -      sprintf (FileNameArry, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
> +      snprintf (FileNameArry, MAX_FILENAME_LEN, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
>        FfsFile = fopen (FileNameArry, "rb");
>        Status = ReadFfsHeader (FfsFile, (UINT32 *)&FileSize);
>        if (EFI_ERROR (Status)) {
I got a new warning after applying this patch:

BinaryParse.c: In function ‘FindFileInFolder’:
BinaryParse.c:1243:54: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 199 [-Werror=format-truncation=]
 1243 |       snprintf (FileNameArry, MAX_FILENAME_LEN, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
      |                                                      ^~
BinaryParse.c:1243:7: note: ‘snprintf’ output 2 or more bytes (assuming 257) into a destination of size 200
 1243 |       snprintf (FileNameArry, MAX_FILENAME_LEN, "%s%c%s", FolderName, OS_SEP, pDirent->d_name);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Gary Lin

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43438): https://edk2.groups.io/g/devel/message/43438
Mute This Topic: https://groups.io/mt/32403705/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-