Re: [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf OptionROM override

["Carsey, Jaben" <jaben.carsey@intel.com>]

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Gao, Liming
> Sent: Sunday, October 07, 2018 6:42 PM
> To: Tomas Pilar (tpilar) <tpilar@solarflare.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf
> OptionROM override
> 
> Pilar:
>   The change is good. Could you also update INF and FDF spec for this usage?
> If you don't know how to update INF and FDF spec, please submit BZ. I will
> provide the spec patch.
> 
>   Reviewed-by: Liming Gao <liming.gao@intel.com>
> 
> Thanks
> Liming
> >-----Original Message-----
> >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> >Tomas Pilar (tpilar)
> >Sent: Tuesday, October 02, 2018 10:46 PM
> >To: edk2-devel@lists.01.org
> >Subject: [edk2] [PATCH] BaseTools: Allow multiple PciDeviceId in Fdf
> >OptionROM override
> >
> >Contributed-under: TianoCore Contribution Agreement 1.1
> >Signed-off-by: Tomas Pilar <tpilar@solarflare.com>
> >---
> > BaseTools/Source/Python/GenFds/FdfParser.py | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> >diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py
> >b/BaseTools/Source/Python/GenFds/FdfParser.py
> >index 63687e98bb..a65f2cfd2d 100644
> >--- a/BaseTools/Source/Python/GenFds/FdfParser.py
> >+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
> >@@ -4469,10 +4469,15 @@ class FdfParser:
> >                 if self.__IsKeyword( "PCI_DEVICE_ID"):
> >                     if not self.__IsToken( "="):
> >                         raise Warning("expected '='", self.FileName,
> >self.CurrentLineNumber)
> >-                    if not self.__GetNextHexNumber():
> >-                        raise Warning("expected Hex device id", self.FileName,
> >self.CurrentLineNumber)
> >
> >-                    Overrides.PciDeviceId = self.__Token
> >+                    # Get a list of PCI IDs
> >+                    Overrides.PciDeviceId = ""
> >+
> >+                    while self.__GetNextHexNumber():
> >+                        Overrides.PciDeviceId += " " + self.__Token

Can we change to minimize looping string concatenation here?  This in a loop will cause lots of memory allocation/deallocation and slow things down.

Maybe : 
Overrides.PciDeviceId = "{} {}".format(Overrides.PciDeviceId, self.__Token)


> >+
> >+                    if not Overrides.PciDeviceId:
> >+                        raise Warning("expected one or more Hex device ids",
> >self.FileName, self.CurrentLineNumber)
> >                     continue
> >
> >                 if self.__IsKeyword( "PCI_REVISION"):
> >--
> >2.17.1
> >
> >_______________________________________________
> >edk2-devel mailing list
> >edk2-devel@lists.01.org
> >https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel