Re: [Patch 3/4 V3] BaseTools: Update Gensec to set PROCESSING_REQUIRED value

["Zhu, Yonghong" <yonghong.zhu@intel.com>]

Hi Leif,

Thanks. I just sent out the patch to fix this bug. Could you help to try it?

Best Regards,
Zhu Yonghong


-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@linaro.org] 
Sent: Tuesday, December 05, 2017 7:53 PM
To: Zhu, Yonghong <yonghong.zhu@intel.com>
Cc: edk2-devel@lists.01.org; Feng, YunhuaX <yunhuax.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2] [Patch 3/4 V3] BaseTools: Update Gensec to set PROCESSING_REQUIRED value

This patch has broken BaseTools build with GCC on master:

make -C GenSec
make[2]: Entering directory '/work/git/edk2/BaseTools/Source/C/GenSec'
gcc  -c -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-unused-result -nostdlib -c -g  -I .. -I ../Include/Common -I ../Include/ -I ../Include/IndustryStandard -I ../Common/ -I .. -I . -I ../Include/X64/  -O2 GenSec.c -o GenSec.o In file included from ../Include/Common/UefiBaseTypes.h:19:0,
                 from GenSec.c:20:
GenSec.c: In function ‘main’:
../Include/Common/BaseTypes.h:178:38: error: overflow in implicit constant conversion [-Werror=overflow]
 #define ENCODE_ERROR(a)              (MAX_BIT | (a))
                                      ^
../Include/Common/BaseTypes.h:204:38: note: in expansion of macro ‘ENCODE_ERROR’
 #define RETURN_ABORTED               ENCODE_ERROR (21)
                                      ^~~~~~~~~~~~
../Include/Common/UefiBaseTypes.h:119:35: note: in expansion of macro ‘RETURN_ABORTED’
 #define EFI_ABORTED               RETURN_ABORTED              
                                   ^~~~~~~~~~~~~~
GenSec.c:1329:16: note: in expansion of macro ‘EFI_ABORTED’
         return EFI_ABORTED;
                ^~~~~~~~~~~
../Include/Common/BaseTypes.h:178:38: error: overflow in implicit constant conversion [-Werror=overflow]
 #define ENCODE_ERROR(a)              (MAX_BIT | (a))
                                      ^
../Include/Common/BaseTypes.h:204:38: note: in expansion of macro ‘ENCODE_ERROR’
 #define RETURN_ABORTED               ENCODE_ERROR (21)
                                      ^~~~~~~~~~~~
../Include/Common/UefiBaseTypes.h:119:35: note: in expansion of macro ‘RETURN_ABORTED’
 #define EFI_ABORTED               RETURN_ABORTED              
                                   ^~~~~~~~~~~~~~
GenSec.c:1343:16: note: in expansion of macro ‘EFI_ABORTED’
         return EFI_ABORTED;
                ^~~~~~~~~~~
GenSec.c:1354:21: error: pointer targets in passing argument 1 of ‘strcasecmp’ differ in signedness [-Werror=pointer-sign]
         if (stricmp(DummyFileBuffer, InFileBuffer + (InFileSize - DummyFileSize)) == 0){
                     ^~~~~~~~~~~~~~~
In file included from GenSec.c:17:0:
/usr/include/string.h:529:12: note: expected ‘const char *’ but argument is of type ‘UINT8 * {aka unsigned char *}’
 extern int strcasecmp (const char *__s1, const char *__s2)
            ^~~~~~~~~~
GenSec.c:1354:38: error: pointer targets in passing argument 2 of ‘strcasecmp’ differ in signedness [-Werror=pointer-sign]
         if (stricmp(DummyFileBuffer, InFileBuffer + (InFileSize - DummyFileSize)) == 0){
                                      ^~~~~~~~~~~~ In file included from GenSec.c:17:0:
/usr/include/string.h:529:12: note: expected ‘const char *’ but argument is of type ‘UINT8 * {aka unsigned char *}’
 extern int strcasecmp (const char *__s1, const char *__s2)
            ^~~~~~~~~~
cc1: all warnings being treated as errors
../Makefiles/footer.makefile:27: recipe for target 'GenSec.o' failed
make[2]: *** [GenSec.o] Error 1
make[2]: Leaving directory '/work/git/edk2/BaseTools/Source/C/GenSec'
GNUmakefile:84: recipe for target 'GenSec' failed
make[1]: *** [GenSec] Error 2
make[1]: Leaving directory '/work/git/edk2/BaseTools/Source/C'
GNUmakefile:25: recipe for target 'Source/C' failed
make: *** [Source/C] Error 2
make: Leaving directory '/work/git/edk2/BaseTools'


On Wed, Nov 29, 2017 at 10:02:05PM +0800, Yonghong Zhu wrote:
> This patch add new option --dummy file, and we compare the dummpy file 
> with input file to decide whether we need to set PROCESSING_REQUIRED 
> value.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yunhua Feng <yunhuax.feng@intel.com>
> ---
>  BaseTools/Source/C/GenSec/GenSec.c | 74 
> ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/BaseTools/Source/C/GenSec/GenSec.c 
> b/BaseTools/Source/C/GenSec/GenSec.c
> index d9cdc1f..904926c 100644
> --- a/BaseTools/Source/C/GenSec/GenSec.c
> +++ b/BaseTools/Source/C/GenSec/GenSec.c
> @@ -185,10 +185,13 @@ Returns:
>                          used in Ver section.\n");
>    fprintf (stdout, "  --sectionalign SectionAlign\n\
>                          SectionAlign points to section alignment, which support\n\
>                          the alignment scope 1~16M. It is specified in same\n\
>                          order that the section file is input.\n");
> +  fprintf (stdout, "  --dummy dummyfile\n\
> +                        compare dummpyfile with input_file to decide whether\n\
> +                        need to set PROCESSING_REQUIRED 
> + attribute.\n");
>    fprintf (stdout, "  -v, --verbose         Turn on verbose output with informational messages.\n");
>    fprintf (stdout, "  -q, --quiet           Disable all messages except key message and fatal error\n");
>    fprintf (stdout, "  -d, --debug level     Enable debug messages, at input debug level.\n");
>    fprintf (stdout, "  --version             Show program's version number and exit.\n");
>    fprintf (stdout, "  -h, --help            Show this help message and exit.\n");
> @@ -1026,10 +1029,17 @@ Returns:
>    EFI_STATUS                Status;
>    UINT64                    LogLevel;
>    UINT32                    *InputFileAlign;
>    UINT32                    InputFileAlignNum;
>    EFI_COMMON_SECTION_HEADER *SectionHeader;
> +  CHAR8                     *DummyFileName;
> +  FILE                      *DummyFile;
> +  UINTN                     DummyFileSize;
> +  UINT8                     *DummyFileBuffer;
> +  FILE                      *InFile;
> +  UINT8                     *InFileBuffer;
> +  UINTN                     InFileSize;
>  
>    InputFileAlign        = NULL;
>    InputFileAlignNum     = 0;
>    InputFileName         = NULL;
>    OutputFileName        = NULL;
> @@ -1047,10 +1057,17 @@ Returns:
>    Status                = STATUS_SUCCESS;
>    LogLevel              = 0;
>    SectGuidHeaderLength  = 0;
>    VersionSect           = NULL;
>    UiSect                = NULL;
> +  DummyFileSize         = 0;
> +  DummyFileName         = NULL;
> +  DummyFile             = NULL;
> +  DummyFileBuffer       = NULL;
> +  InFile                = NULL;
> +  InFileSize            = 0;
> +  InFileBuffer          = NULL;
>    
>    SetUtilityName (UTILITY_NAME);
>    
>    if (argc == 1) {
>      Error (NULL, 0, 1001, "Missing options", "No options input"); @@ 
> -1117,10 +1134,20 @@ Returns:
>        }
>        argc -= 2;
>        argv += 2;
>        continue;
>      }
> +    if (stricmp (argv[0], "--dummy") == 0) {
> +      DummyFileName = argv[1];
> +      if (DummyFileName == NULL) {
> +        Error (NULL, 0, 1003, "Invalid option value", "Dummy file can't be NULL");
> +        goto Finish;
> +      }
> +      argc -= 2;
> +      argv += 2;
> +      continue;
> +    }
>  
>      if ((stricmp (argv[0], "-r") == 0) || (stricmp (argv[0], "--attributes") == 0)) {
>        if (argv[1] == NULL) {
>          Error (NULL, 0, 1003, "Invalid option value", "Guid section attributes can't be NULL");
>          goto Finish;
> @@ -1290,10 +1317,57 @@ Returns:
>      goto Finish;
>    }
>  
>    VerboseMsg ("%s tool start.", UTILITY_NAME);
>  
> +  if (DummyFileName != NULL) {
> +      //
> +      // Open file and read contents
> +      //
> +      DummyFile = fopen (LongFilePath (DummyFileName), "rb");
> +      if (DummyFile == NULL) {
> +        Error (NULL, 0, 0001, "Error opening file", DummyFileName);
> +        return EFI_ABORTED;
> +      }
> +
> +      fseek (DummyFile, 0, SEEK_END);
> +      DummyFileSize = ftell (DummyFile);
> +      fseek (DummyFile, 0, SEEK_SET);
> +      DummyFileBuffer = (UINT8 *) malloc (DummyFileSize);
> +      fread(DummyFileBuffer, 1, DummyFileSize, DummyFile);
> +      fclose(DummyFile);
> +      DebugMsg (NULL, 0, 9, "Dummy files", "the dummy file name is %s 
> + and the size is %u bytes", DummyFileName, (unsigned) DummyFileSize);
> +
> +      InFile = fopen(LongFilePath(InputFileName[0]), "rb");
> +      if (InFile == NULL) {
> +        Error (NULL, 0, 0001, "Error opening file", InputFileName[0]);
> +        return EFI_ABORTED;
> +      }
> +
> +      fseek (InFile, 0, SEEK_END);
> +      InFileSize = ftell (InFile);
> +      fseek (InFile, 0, SEEK_SET);
> +      InFileBuffer = (UINT8 *) malloc (InFileSize);
> +      fread(InFileBuffer, 1, InFileSize, InFile);
> +      fclose(InFile);
> +      DebugMsg (NULL, 0, 9, "Input files", "the input file name is %s and the size is %u bytes", InputFileName[0], (unsigned) InFileSize);
> +      if (InFileSize > DummyFileSize){
> +        if (stricmp(DummyFileBuffer, InFileBuffer + (InFileSize - DummyFileSize)) == 0){
> +          SectGuidHeaderLength = InFileSize - DummyFileSize;
> +        }
> +      }
> +      if (SectGuidHeaderLength == 0) {
> +        SectGuidAttribute |= EFI_GUIDED_SECTION_PROCESSING_REQUIRED;
> +      }
> +      if (DummyFileBuffer != NULL) {
> +        free (DummyFileBuffer);
> +      }
> +      if (InFileBuffer != NULL) {
> +        free (InFileBuffer);
> +      }
> +    }
> +
>    //
>    // Parse all command line parameters to get the corresponding section type.
>    //
>    VerboseMsg ("Section type is %s", SectionName);
>    if (SectionName == NULL) {
> --
> 2.6.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel