[PATCH 0/2] IntelFsp2Pkg: Coverity scan flags issues

[PATCH 0/2] IntelFsp2Pkg: Coverity scan flags issues

[Chiu, Chasel]

GenCfgOpt.py:
Issue was: invalid_operation: Invalid operation on
null-like value "Base".
Fixed it by initializing Base to 0 before entering
while loop.

SplitFspBin.py:
Issues were:
1. copy_paste_error: PeOptHdr vs PePlusOptHdr.
2. invalid_operation: Invalid operation on
null-like value "roffset".
3. invalid_operation: Invalid operation on
null-like value "rsize".
Fixed them by initializing roffset and rsize as PE32
image type and overriding them when PE32+ header found.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>

Chasel Chiu (2):
  IntelFsp2Pkg/GenCfgOpt.py: Coverity scan flags issues.
  IntelFsp2Pkg/SplitFspBin.py: Coverity scan flags issues.

 IntelFsp2Pkg/Tools/GenCfgOpt.py   |  3 ++-
 IntelFsp2Pkg/Tools/SplitFspBin.py | 10 +++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

-- 
2.13.3.windows.1

[PATCH 1/2] IntelFsp2Pkg/GenCfgOpt.py: Coverity scan flags issues.

[Chiu, Chasel]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1706

Issue was: invalid_operation: Invalid operation on
null-like value "Base".
Fixed it by initializing Base to 0 before entering
while loop.

Test:
Compared script output before and after this patch and
the result is identical.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 IntelFsp2Pkg/Tools/GenCfgOpt.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
index 2fa7582d85..d1d6901bc3 100644
--- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
+++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
@@ -1,6 +1,6 @@
 ## @ GenCfgOpt.py
 #
-# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -439,6 +439,7 @@ EndList
 
         MaxAlign = 32   #Default align to 32, but if there are 64 bit unit, align to 64
         SizeAlign = 0   #record the struct max align
+        Base = 0        #Starting offset of sub-structure.
         while len(DscLines):
             DscLine  = DscLines.pop(0).strip()
             Handle   = False
-- 
2.13.3.windows.1

[PATCH 2/2] IntelFsp2Pkg/SplitFspBin.py: Coverity scan flags issues.

[Chiu, Chasel]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1706

Issues were:
1. copy_paste_error: PeOptHdr vs PePlusOptHdr.
2. invalid_operation: Invalid operation on
null-like value "roffset".
3. invalid_operation: Invalid operation on
null-like value "rsize".

Fixed them by initializing roffset and rsize as PE32
image type and overriding them when PE32+ header found.

Test:
Compared script output before and after this patch and
the result is identical.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
 IntelFsp2Pkg/Tools/SplitFspBin.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py b/IntelFsp2Pkg/Tools/SplitFspBin.py
index 39a7ea7460..3c0d5af1b6 100644
--- a/IntelFsp2Pkg/Tools/SplitFspBin.py
+++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
@@ -1,6 +1,6 @@
 ## @ FspTool.py
 #
-# Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -619,10 +619,10 @@ class PeTeImage:
             rsize   = self.TeHdr.DataDirectoryBaseReloc.Size
             roffset = sizeof(self.TeHdr) - self.TeHdr.StrippedSize + self.TeHdr.DataDirectoryBaseReloc.VirtualAddress
         else:
-            if self.PeHdr.OptionalHeader.PeOptHdr.Magic == 0x10b: # PE32 image
-                rsize   = self.PeHdr.OptionalHeader.PeOptHdr.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY.BASERELOC].Size
-                roffset = self.PeHdr.OptionalHeader.PeOptHdr.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY.BASERELOC].VirtualAddress
-            if self.PeHdr.OptionalHeader.PeOptHdr.Magic == 0x20b: # PE32+ image
+            # Assuming PE32 image type (self.PeHdr.OptionalHeader.PeOptHdr.Magic == 0x10b)
+            rsize   = self.PeHdr.OptionalHeader.PeOptHdr.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY.BASERELOC].Size
+            roffset = self.PeHdr.OptionalHeader.PeOptHdr.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY.BASERELOC].VirtualAddress
+            if self.PeHdr.OptionalHeader.PePlusOptHdr.Magic == 0x20b: # PE32+ image
                 rsize   = self.PeHdr.OptionalHeader.PePlusOptHdr.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY.BASERELOC].Size
                 roffset = self.PeHdr.OptionalHeader.PePlusOptHdr.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY.BASERELOC].VirtualAddress
 
-- 
2.13.3.windows.1

Re: [PATCH 0/2] IntelFsp2Pkg: Coverity scan flags issues

[Zeng, Star]

Reviewed-by: Star Zeng <star.zeng@intel.com>

> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Monday, February 10, 2020 11:53 AM
> To: devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 0/2] IntelFsp2Pkg: Coverity scan flags issues
> 
> GenCfgOpt.py:
> Issue was: invalid_operation: Invalid operation on null-like value "Base".
> Fixed it by initializing Base to 0 before entering while loop.
> 
> SplitFspBin.py:
> Issues were:
> 1. copy_paste_error: PeOptHdr vs PePlusOptHdr.
> 2. invalid_operation: Invalid operation on null-like value "roffset".
> 3. invalid_operation: Invalid operation on null-like value "rsize".
> Fixed them by initializing roffset and rsize as PE32 image type and overriding
> them when PE32+ header found.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> 
> Chasel Chiu (2):
>   IntelFsp2Pkg/GenCfgOpt.py: Coverity scan flags issues.
>   IntelFsp2Pkg/SplitFspBin.py: Coverity scan flags issues.
> 
>  IntelFsp2Pkg/Tools/GenCfgOpt.py   |  3 ++-
>  IntelFsp2Pkg/Tools/SplitFspBin.py | 10 +++++-----
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> --
> 2.13.3.windows.1

Re: [edk2-devel] [PATCH 1/2] IntelFsp2Pkg/GenCfgOpt.py: Coverity scan flags issues.

[Philippe Mathieu-Daudé]

On 2/10/20 4:53 AM, Chiu, Chasel wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1706
> 
> Issue was: invalid_operation: Invalid operation on
> null-like value "Base".
> Fixed it by initializing Base to 0 before entering
> while loop.
> 
> Test:
> Compared script output before and after this patch and
> the result is identical.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>   IntelFsp2Pkg/Tools/GenCfgOpt.py | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> index 2fa7582d85..d1d6901bc3 100644
> --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> @@ -1,6 +1,6 @@
>   ## @ GenCfgOpt.py
>   #
> -# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
>   ##
> @@ -439,6 +439,7 @@ EndList
>   
>           MaxAlign = 32   #Default align to 32, but if there are 64 bit unit, align to 64
>           SizeAlign = 0   #record the struct max align
> +        Base = 0        #Starting offset of sub-structure.

Are you sure this has to be initialized only once before the while 
statement, and not within the 'Check VPD/UPD' leaf?

>           while len(DscLines):
>               DscLine  = DscLines.pop(0).strip()
>               Handle   = False
> 

Re: [edk2-devel] [PATCH 1/2] IntelFsp2Pkg/GenCfgOpt.py: Coverity scan flags issues.

[Chiu, Chasel]

Hi Philippe,

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Philippe
> Mathieu-Daude
> Sent: Monday, February 10, 2020 4:36 PM
> To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@intel.com>
> Cc: Ma, Maurice <maurice.ma@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2-devel] [PATCH 1/2] IntelFsp2Pkg/GenCfgOpt.py: Coverity
> scan flags issues.
> 
> On 2/10/20 4:53 AM, Chiu, Chasel wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1706
> >
> > Issue was: invalid_operation: Invalid operation on null-like value
> > "Base".
> > Fixed it by initializing Base to 0 before entering while loop.
> >
> > Test:
> > Compared script output before and after this patch and the result is
> > identical.
> >
> > Cc: Maurice Ma <maurice.ma@intel.com>
> > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > ---
> >   IntelFsp2Pkg/Tools/GenCfgOpt.py | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > b/IntelFsp2Pkg/Tools/GenCfgOpt.py index 2fa7582d85..d1d6901bc3 100644
> > --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> > @@ -1,6 +1,6 @@
> >   ## @ GenCfgOpt.py
> >   #
> > -# Copyright (c) 2014 - 2019, Intel Corporation. All rights
> > reserved.<BR>
> > +# Copyright (c) 2014 - 2020, Intel Corporation. All rights
> > +reserved.<BR>
> >   # SPDX-License-Identifier: BSD-2-Clause-Patent
> >   #
> >   ##
> > @@ -439,6 +439,7 @@ EndList
> >
> >           MaxAlign = 32   #Default align to 32, but if there are 64 bit
> unit, align to 64
> >           SizeAlign = 0   #record the struct max align
> > +        Base = 0        #Starting offset of sub-structure.
> 
> Are you sure this has to be initialized only once before the while statement,
> and not within the 'Check VPD/UPD' leaf?
> 

Yes, initializing 'Base' before entering while loop is good enough.
This optional feature (auto-assigning UPD element offset) is implemented basing on FSP 2.0 spec and offset 0 in the UPD structure is always sub-structure field "FSP_UPD_HEADER. Signature", so 'Base' will be always initialized to sub-structure beginning offset before it is actually consumed.

Thanks,
Chasel

Code snippet line#707:
                        if (ConfigDict['embed'].find(':START') != -1):  <== offset 0 will always meet this condition and assign Base to ItemOffset, which is 0 too.
                            Base = ItemOffset
                        SubOffset = ItemOffset - Base
                        SubRemainder = SubOffset % Unit
                        if SubRemainder:
                            Diff = Unit - SubRemainder
                            Offset = Offset + Diff
                            ItemOffset = ItemOffset + Diff




> >           while len(DscLines):
> >               DscLine  = DscLines.pop(0).strip()
> >               Handle   = False
> >
> 
> 
> 

Re: [PATCH 0/2] IntelFsp2Pkg: Coverity scan flags issues

[Nate DeSimone]

For the patch series...

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com> 
Sent: Sunday, February 9, 2020 7:53 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 0/2] IntelFsp2Pkg: Coverity scan flags issues

GenCfgOpt.py:
Issue was: invalid_operation: Invalid operation on null-like value "Base".
Fixed it by initializing Base to 0 before entering while loop.

SplitFspBin.py:
Issues were:
1. copy_paste_error: PeOptHdr vs PePlusOptHdr.
2. invalid_operation: Invalid operation on null-like value "roffset".
3. invalid_operation: Invalid operation on null-like value "rsize".
Fixed them by initializing roffset and rsize as PE32 image type and overriding them when PE32+ header found.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>

Chasel Chiu (2):
  IntelFsp2Pkg/GenCfgOpt.py: Coverity scan flags issues.
  IntelFsp2Pkg/SplitFspBin.py: Coverity scan flags issues.

 IntelFsp2Pkg/Tools/GenCfgOpt.py   |  3 ++-
 IntelFsp2Pkg/Tools/SplitFspBin.py | 10 +++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

--
2.13.3.windows.1