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

[Chiu, Chasel]

Hi Philippe,

> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Philippe
> Mathieu-Daude
> Sent: Monday, February 10, 2020 4:36 PM
> To: devel at edk2.groups.io; Chiu, Chasel <chasel.chiu at intel.com>
> Cc: Ma, Maurice <maurice.ma at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>; Zeng, Star <star.zeng at 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 at intel.com>
> > Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> > Cc: Star Zeng <star.zeng at intel.com>
> > Signed-off-by: Chasel Chiu <chasel.chiu at 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
> >
> 
> 
> 


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

View/Reply Online (#54135): https://edk2.groups.io/g/devel/message/54135
Mute This Topic: https://groups.io/mt/71127615/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-