Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.
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 ; devel@edk2.groups.io;
>leif.lindh...@linaro.org; Kinney, Michael D
>Cc: Gao, Zhichao ; Feng, Bob C
>; Andrew Fish ; Laszlo Ersek
>
>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.lindh...@linaro.org
>> Cc: Gao, Zhichao ; Feng, Bob C
>> ; Andrew Fish ;
>> Laszlo Ersek ; Kinney, Michael D
>>
>> 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
>>
>> > Cc: Gao, Zhichao ; Feng, Bob C
>> > ; Andrew Fish ;
>> Laszlo Ersek
>> > ; Kinney, Michael D
>>
>> > 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
>> > >
>> > > 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
>> > > Cc: Bob Feng
>> > > ---
>> > > 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");
>> >
Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.
On Wed, Jul 10, 2019 at 01:42:44PM +, 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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
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.lindh...@linaro.org
> Cc: Gao, Zhichao ; Feng, Bob C
> ; Andrew Fish ;
> Laszlo Ersek ; Kinney, Michael D
>
> 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
>
> > Cc: Gao, Zhichao ; Feng, Bob C
> > ; Andrew Fish ;
> Laszlo Ersek
> > ; Kinney, Michael D
>
> > 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
> > >
> > > 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
> > > Cc: Bob Feng
> > > ---
> > > 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.
>
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
> Cc: Gao, Zhichao ; Feng, Bob C ;
> Andrew Fish ; Laszlo Ersek
> ; Kinney, Michael D
> 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
> >
> > 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
> > Cc: Bob Feng
> > ---
> > 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 a
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
>
> 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
> Cc: Bob Feng
> ---
> 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
Re: [edk2-devel] [Patch v3] BaseTools: Fix GCC compiler failure in new added tools.
On Tue, Jul 09, 2019 at 05:53:33PM +0800, Liming Gao wrote:
> From: gaozhic
>
> GCC 7 or 8 reports some warnings in new added FCE/FMMT/BlmLib.
>
> Signed-off-by: Liming Gao
> Cc: Bob Feng
> ---
> 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 *));
>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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
