public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif.lindholm@linaro.org>
To: devel@edk2.groups.io, liming.gao@intel.com
Cc: gaozhic <zhichao.gao@intel.com>, Bob Feng <bob.c.feng@intel.com>,
	Andrew Fish <afish@apple.com>, Laszlo Ersek <lersek@redhat.com>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.
Date: Tue, 9 Jul 2019 19:26:02 +0100	[thread overview]
Message-ID: <20190709182559.to6n5pifdkegc7yy@bivouac.eciton.net> (raw)
In-Reply-To: <1562666013-13188-1-git-send-email-liming.gao@intel.com>

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

  parent reply	other threads:[~2019-07-09 18:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-09  9:53 [Patch v3] BaseTools: Fix GCC compiler failure in new added tools Liming Gao
2019-07-09 10:19 ` [edk2-devel] " Gary Lin
2019-07-09 18:26 ` Leif Lindholm [this message]
2019-07-10 13:42   ` Liming Gao
2019-07-10 20:10     ` Michael D Kinney
2019-07-11  1:47       ` Liming Gao
2019-07-10 20:57     ` Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190709182559.to6n5pifdkegc7yy@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox