* [PATCH v1 0/4] BaseTools: remove code without affect
@ 2018-03-27 23:42 Jaben Carsey
2018-03-27 23:42 ` [PATCH v1 1/4] BaseTools: remove irrelevant code Jaben Carsey
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Jaben Carsey @ 2018-03-27 23:42 UTC (permalink / raw)
To: edk2-devel
this is a refactoring of the code to make it more readable and less wasteful
*** BLURB HERE ***
Jaben Carsey (4):
BaseTools: remove irrelevant code
BaseTools: expression can use single in instead of 3 API calls.
BaseTools: move regular expression compile out of function call.
BaseTools: dont use enumerate when un-needed
BaseTools/Source/Python/Common/Expression.py | 73 ++++----------------
BaseTools/Source/Python/Common/Misc.py | 2 +-
BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py | 2 +-
3 files changed, 16 insertions(+), 61 deletions(-)
--
2.16.2.windows.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 1/4] BaseTools: remove irrelevant code
2018-03-27 23:42 [PATCH v1 0/4] BaseTools: remove code without affect Jaben Carsey
@ 2018-03-27 23:42 ` Jaben Carsey
2018-03-28 2:17 ` Zhu, Yonghong
2018-03-27 23:42 ` [PATCH v1 2/4] BaseTools: expression can use single in instead of 3 API calls Jaben Carsey
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Jaben Carsey @ 2018-03-27 23:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Liming Gao, Yonghong Zhu
Since PcdValue is a string, no need to test it's type() for string
Also remove the block used if it's a list (which is never is)
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/Python/Common/Expression.py | 55 ++------------------
1 file changed, 5 insertions(+), 50 deletions(-)
diff --git a/BaseTools/Source/Python/Common/Expression.py b/BaseTools/Source/Python/Common/Expression.py
index 4f0f377f3788..b70f7da1233e 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -815,57 +815,12 @@ class ValueExpressionEx(ValueExpression):
except BadExpression, Value:
if self.PcdType in ['UINT8', 'UINT16', 'UINT32', 'UINT64', 'BOOLEAN']:
PcdValue = PcdValue.strip()
- if type(PcdValue) == type('') and PcdValue.startswith('{') and PcdValue.endswith('}'):
+ if PcdValue.startswith('{') and PcdValue.endswith('}'):
PcdValue = SplitPcdValueString(PcdValue[1:-1])
- if type(PcdValue) == type([]):
- TmpValue = 0
- Size = 0
- ValueType = ''
- for Item in PcdValue:
- Item = Item.strip()
- if Item.startswith('UINT8'):
- ItemSize = 1
- ValueType = 'UINT8'
- elif Item.startswith('UINT16'):
- ItemSize = 2
- ValueType = 'UINT16'
- elif Item.startswith('UINT32'):
- ItemSize = 4
- ValueType = 'UINT32'
- elif Item.startswith('UINT64'):
- ItemSize = 8
- ValueType = 'UINT64'
- elif Item.startswith('"') or Item.startswith("'") or Item.startswith('L'):
- ItemSize = 0
- ValueType = 'VOID*'
- else:
- ItemSize = 0
- ValueType = 'UINT8'
- Item = ValueExpressionEx(Item, ValueType, self._Symb)(True)
-
- if ItemSize == 0:
- try:
- tmpValue = int(Item, 16) if Item.upper().startswith('0X') else int(Item, 0)
- if tmpValue > 255:
- raise BadExpression("Byte array number %s should less than 0xFF." % Item)
- except BadExpression, Value:
- raise BadExpression(Value)
- except ValueError:
- pass
- ItemValue, ItemSize = ParseFieldValue(Item)
- else:
- ItemValue = ParseFieldValue(Item)[0]
-
- if type(ItemValue) == type(''):
- ItemValue = int(ItemValue, 16) if ItemValue.startswith('0x') else int(ItemValue)
-
- TmpValue = (ItemValue << (Size * 8)) | TmpValue
- Size = Size + ItemSize
- else:
- try:
- TmpValue, Size = ParseFieldValue(PcdValue)
- except BadExpression, Value:
- raise BadExpression("Type: %s, Value: %s, %s" % (self.PcdType, PcdValue, Value))
+ try:
+ TmpValue, Size = ParseFieldValue(PcdValue)
+ except BadExpression, Value:
+ raise BadExpression("Type: %s, Value: %s, %s" % (self.PcdType, PcdValue, Value))
if type(TmpValue) == type(''):
try:
TmpValue = int(TmpValue)
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 2/4] BaseTools: expression can use single in instead of 3 API calls.
2018-03-27 23:42 [PATCH v1 0/4] BaseTools: remove code without affect Jaben Carsey
2018-03-27 23:42 ` [PATCH v1 1/4] BaseTools: remove irrelevant code Jaben Carsey
@ 2018-03-27 23:42 ` Jaben Carsey
2018-03-29 2:55 ` Zhu, Yonghong
2018-03-27 23:42 ` [PATCH v1 3/4] BaseTools: move regular expression compile out of function call Jaben Carsey
2018-03-27 23:42 ` [PATCH v1 4/4] BaseTools: dont use enumerate when un-needed Jaben Carsey
3 siblings, 1 reply; 11+ messages in thread
From: Jaben Carsey @ 2018-03-27 23:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Liming Gao, Yonghong Zhu
change 3 StartsWith() calls to a single 'in' operation.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/Python/Common/Expression.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/BaseTools/Source/Python/Common/Expression.py b/BaseTools/Source/Python/Common/Expression.py
index b70f7da1233e..3f2b43118553 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -948,7 +948,7 @@ class ValueExpressionEx(ValueExpression):
Item = '0x%x' % TmpValue if type(TmpValue) != type('') else TmpValue
if ItemSize == 0:
ItemValue, ItemSize = ParseFieldValue(Item)
- if not (Item.startswith('"') or Item.startswith('L') or Item.startswith('{')) and ItemSize > 1:
+ if Item[0] not in ['"','L','{'] and ItemSize > 1:
raise BadExpression("Byte array number %s should less than 0xFF." % Item)
else:
ItemValue = ParseFieldValue(Item)[0]
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 3/4] BaseTools: move regular expression compile out of function call.
2018-03-27 23:42 [PATCH v1 0/4] BaseTools: remove code without affect Jaben Carsey
2018-03-27 23:42 ` [PATCH v1 1/4] BaseTools: remove irrelevant code Jaben Carsey
2018-03-27 23:42 ` [PATCH v1 2/4] BaseTools: expression can use single in instead of 3 API calls Jaben Carsey
@ 2018-03-27 23:42 ` Jaben Carsey
2018-03-29 2:55 ` Zhu, Yonghong
2018-03-27 23:42 ` [PATCH v1 4/4] BaseTools: dont use enumerate when un-needed Jaben Carsey
3 siblings, 1 reply; 11+ messages in thread
From: Jaben Carsey @ 2018-03-27 23:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Liming Gao, Yonghong Zhu
move to the root of the file and dont recompile.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/Python/Common/Expression.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/BaseTools/Source/Python/Common/Expression.py b/BaseTools/Source/Python/Common/Expression.py
index 3f2b43118553..340c50ebe00f 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -41,6 +41,8 @@ ERR_EMPTY_EXPR = 'Empty expression is not allowed.'
ERR_IN_OPERAND = 'Macro after IN operator can only be: $(FAMILY), $(ARCH), $(TOOL_CHAIN_TAG) and $(TARGET).'
__ValidString = re.compile(r'[_a-zA-Z][_0-9a-zA-Z]*$')
+_ReLabel = re.compile('LABEL\((\w+)\)')
+_ReOffset = re.compile('OFFSET_OF\((\w+)\)')
## SplitString
# Split string to list according double quote
@@ -853,13 +855,11 @@ class ValueExpressionEx(ValueExpression):
PcdValueList = SplitPcdValueString(PcdValue.strip()[1:-1])
LabelDict = {}
NewPcdValueList = []
- ReLabel = re.compile('LABEL\((\w+)\)')
- ReOffset = re.compile('OFFSET_OF\((\w+)\)')
LabelOffset = 0
for Index, Item in enumerate(PcdValueList):
# compute byte offset of every LABEL
- LabelList = ReLabel.findall(Item)
- Item = ReLabel.sub('', Item)
+ LabelList = _ReLabel.findall(Item)
+ Item = _ReLabel.sub('', Item)
Item = Item.strip()
if LabelList:
for Label in LabelList:
@@ -886,11 +886,11 @@ class ValueExpressionEx(ValueExpression):
# for LABEL parse
Item = Item.strip()
try:
- Item = ReLabel.sub('', Item)
+ Item = _ReLabel.sub('', Item)
except:
pass
try:
- OffsetList = ReOffset.findall(Item)
+ OffsetList = _ReOffset.findall(Item)
except:
pass
for Offset in OffsetList:
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 4/4] BaseTools: dont use enumerate when un-needed
2018-03-27 23:42 [PATCH v1 0/4] BaseTools: remove code without affect Jaben Carsey
` (2 preceding siblings ...)
2018-03-27 23:42 ` [PATCH v1 3/4] BaseTools: move regular expression compile out of function call Jaben Carsey
@ 2018-03-27 23:42 ` Jaben Carsey
2018-03-29 2:55 ` Zhu, Yonghong
3 siblings, 1 reply; 11+ messages in thread
From: Jaben Carsey @ 2018-03-27 23:42 UTC (permalink / raw)
To: edk2-devel; +Cc: Liming Gao, Yonghong Zhu
Since we only use the item from the list and not the numeric value,
dont bother with enumerate()
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/Python/Common/Expression.py | 4 ++--
BaseTools/Source/Python/Common/Misc.py | 2 +-
BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/BaseTools/Source/Python/Common/Expression.py b/BaseTools/Source/Python/Common/Expression.py
index 340c50ebe00f..f28d770aad94 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -856,7 +856,7 @@ class ValueExpressionEx(ValueExpression):
LabelDict = {}
NewPcdValueList = []
LabelOffset = 0
- for Index, Item in enumerate(PcdValueList):
+ for Item in PcdValueList:
# compute byte offset of every LABEL
LabelList = _ReLabel.findall(Item)
Item = _ReLabel.sub('', Item)
@@ -882,7 +882,7 @@ class ValueExpressionEx(ValueExpression):
except:
LabelOffset = LabelOffset + 1
- for Index, Item in enumerate(PcdValueList):
+ for Item in PcdValueList:
# for LABEL parse
Item = Item.strip()
try:
diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py
index 7d44fdcf8ba7..8f479ace4cb1 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -76,7 +76,7 @@ def GetVariableOffset(mapfilepath, efifilepath, varnames):
def _parseForXcode(lines, efifilepath, varnames):
status = 0
ret = []
- for index, line in enumerate(lines):
+ for line in lines:
line = line.strip()
if status == 0 and line == "# Symbols:":
status = 1
diff --git a/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py b/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
index fdad5a44dc3d..0a701158e2c2 100644
--- a/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
+++ b/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
@@ -65,7 +65,7 @@ def parsePcdInfoFromMapFile(mapfilepath, efifilepath):
def _parseForXcode(lines, efifilepath):
status = 0
pcds = []
- for index, line in enumerate(lines):
+ for line in lines:
line = line.strip()
if status == 0 and line == "# Symbols:":
status = 1
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/4] BaseTools: remove irrelevant code
2018-03-27 23:42 ` [PATCH v1 1/4] BaseTools: remove irrelevant code Jaben Carsey
@ 2018-03-28 2:17 ` Zhu, Yonghong
2018-03-28 14:57 ` Carsey, Jaben
2018-03-28 20:22 ` Carsey, Jaben
0 siblings, 2 replies; 11+ messages in thread
From: Zhu, Yonghong @ 2018-03-28 2:17 UTC (permalink / raw)
To: Carsey, Jaben, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
Hi Jaben,
Current we support flexible PCD value format, example: UINT64 type PCD can use {0x1, 2, 'A', UINT16(10), "B"} as its value, so the block can't be removed since it maybe a list after SplitPcdValueString function.
https://bugzilla.tianocore.org/show_bug.cgi?id=541 is the feature request for flexible Pcd value format.
Best Regards,
Zhu Yonghong
-----Original Message-----
From: Carsey, Jaben
Sent: Wednesday, March 28, 2018 7:43 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [PATCH v1 1/4] BaseTools: remove irrelevant code
Since PcdValue is a string, no need to test it's type() for string Also remove the block used if it's a list (which is never is)
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/Python/Common/Expression.py | 55 ++------------------
1 file changed, 5 insertions(+), 50 deletions(-)
diff --git a/BaseTools/Source/Python/Common/Expression.py b/BaseTools/Source/Python/Common/Expression.py
index 4f0f377f3788..b70f7da1233e 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -815,57 +815,12 @@ class ValueExpressionEx(ValueExpression):
except BadExpression, Value:
if self.PcdType in ['UINT8', 'UINT16', 'UINT32', 'UINT64', 'BOOLEAN']:
PcdValue = PcdValue.strip()
- if type(PcdValue) == type('') and PcdValue.startswith('{') and PcdValue.endswith('}'):
+ if PcdValue.startswith('{') and PcdValue.endswith('}'):
PcdValue = SplitPcdValueString(PcdValue[1:-1])
- if type(PcdValue) == type([]):
- TmpValue = 0
- Size = 0
- ValueType = ''
- for Item in PcdValue:
- Item = Item.strip()
- if Item.startswith('UINT8'):
- ItemSize = 1
- ValueType = 'UINT8'
- elif Item.startswith('UINT16'):
- ItemSize = 2
- ValueType = 'UINT16'
- elif Item.startswith('UINT32'):
- ItemSize = 4
- ValueType = 'UINT32'
- elif Item.startswith('UINT64'):
- ItemSize = 8
- ValueType = 'UINT64'
- elif Item.startswith('"') or Item.startswith("'") or Item.startswith('L'):
- ItemSize = 0
- ValueType = 'VOID*'
- else:
- ItemSize = 0
- ValueType = 'UINT8'
- Item = ValueExpressionEx(Item, ValueType, self._Symb)(True)
-
- if ItemSize == 0:
- try:
- tmpValue = int(Item, 16) if Item.upper().startswith('0X') else int(Item, 0)
- if tmpValue > 255:
- raise BadExpression("Byte array number %s should less than 0xFF." % Item)
- except BadExpression, Value:
- raise BadExpression(Value)
- except ValueError:
- pass
- ItemValue, ItemSize = ParseFieldValue(Item)
- else:
- ItemValue = ParseFieldValue(Item)[0]
-
- if type(ItemValue) == type(''):
- ItemValue = int(ItemValue, 16) if ItemValue.startswith('0x') else int(ItemValue)
-
- TmpValue = (ItemValue << (Size * 8)) | TmpValue
- Size = Size + ItemSize
- else:
- try:
- TmpValue, Size = ParseFieldValue(PcdValue)
- except BadExpression, Value:
- raise BadExpression("Type: %s, Value: %s, %s" % (self.PcdType, PcdValue, Value))
+ try:
+ TmpValue, Size = ParseFieldValue(PcdValue)
+ except BadExpression, Value:
+ raise BadExpression("Type: %s, Value: %s, %s" %
+ (self.PcdType, PcdValue, Value))
if type(TmpValue) == type(''):
try:
TmpValue = int(TmpValue)
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/4] BaseTools: remove irrelevant code
2018-03-28 2:17 ` Zhu, Yonghong
@ 2018-03-28 14:57 ` Carsey, Jaben
2018-03-28 20:22 ` Carsey, Jaben
1 sibling, 0 replies; 11+ messages in thread
From: Carsey, Jaben @ 2018-03-28 14:57 UTC (permalink / raw)
To: Zhu, Yonghong, edk2-devel@lists.01.org; +Cc: Gao, Liming
Zhu,
Given that the statement ~1 line above "PcdValue = PcdValue.strip()" will raise exception with a PcdValue that is a list, I don't see how this code could execute.
I agree that it is likely that we think we want this, but something is wrong with it and I assumed that the current behavior was the desired behavior. If I am incorrect, please feel free to disregard this patch.
-Jaben
> -----Original Message-----
> From: Zhu, Yonghong
> Sent: Tuesday, March 27, 2018 7:18 PM
> To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong
> <yonghong.zhu@intel.com>
> Subject: RE: [PATCH v1 1/4] BaseTools: remove irrelevant code
> Importance: High
>
> Hi Jaben,
>
> Current we support flexible PCD value format, example: UINT64 type PCD
> can use {0x1, 2, 'A', UINT16(10), "B"} as its value, so the block can't be
> removed since it maybe a list after SplitPcdValueString function.
> https://bugzilla.tianocore.org/show_bug.cgi?id=541 is the feature request
> for flexible Pcd value format.
>
> Best Regards,
> Zhu Yonghong
>
>
> -----Original Message-----
> From: Carsey, Jaben
> Sent: Wednesday, March 28, 2018 7:43 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong
> <yonghong.zhu@intel.com>
> Subject: [PATCH v1 1/4] BaseTools: remove irrelevant code
>
> Since PcdValue is a string, no need to test it's type() for string Also remove
> the block used if it's a list (which is never is)
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
> ---
> BaseTools/Source/Python/Common/Expression.py | 55 ++------------------
> 1 file changed, 5 insertions(+), 50 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Common/Expression.py
> b/BaseTools/Source/Python/Common/Expression.py
> index 4f0f377f3788..b70f7da1233e 100644
> --- a/BaseTools/Source/Python/Common/Expression.py
> +++ b/BaseTools/Source/Python/Common/Expression.py
> @@ -815,57 +815,12 @@ class ValueExpressionEx(ValueExpression):
> except BadExpression, Value:
> if self.PcdType in ['UINT8', 'UINT16', 'UINT32', 'UINT64', 'BOOLEAN']:
> PcdValue = PcdValue.strip()
> - if type(PcdValue) == type('') and PcdValue.startswith('{') and
> PcdValue.endswith('}'):
> + if PcdValue.startswith('{') and PcdValue.endswith('}'):
> PcdValue = SplitPcdValueString(PcdValue[1:-1])
> - if type(PcdValue) == type([]):
> - TmpValue = 0
> - Size = 0
> - ValueType = ''
> - for Item in PcdValue:
> - Item = Item.strip()
> - if Item.startswith('UINT8'):
> - ItemSize = 1
> - ValueType = 'UINT8'
> - elif Item.startswith('UINT16'):
> - ItemSize = 2
> - ValueType = 'UINT16'
> - elif Item.startswith('UINT32'):
> - ItemSize = 4
> - ValueType = 'UINT32'
> - elif Item.startswith('UINT64'):
> - ItemSize = 8
> - ValueType = 'UINT64'
> - elif Item.startswith('"') or Item.startswith("'") or
> Item.startswith('L'):
> - ItemSize = 0
> - ValueType = 'VOID*'
> - else:
> - ItemSize = 0
> - ValueType = 'UINT8'
> - Item = ValueExpressionEx(Item, ValueType, self._Symb)(True)
> -
> - if ItemSize == 0:
> - try:
> - tmpValue = int(Item, 16) if Item.upper().startswith('0X') else
> int(Item, 0)
> - if tmpValue > 255:
> - raise BadExpression("Byte array number %s should less
> than 0xFF." % Item)
> - except BadExpression, Value:
> - raise BadExpression(Value)
> - except ValueError:
> - pass
> - ItemValue, ItemSize = ParseFieldValue(Item)
> - else:
> - ItemValue = ParseFieldValue(Item)[0]
> -
> - if type(ItemValue) == type(''):
> - ItemValue = int(ItemValue, 16) if ItemValue.startswith('0x')
> else int(ItemValue)
> -
> - TmpValue = (ItemValue << (Size * 8)) | TmpValue
> - Size = Size + ItemSize
> - else:
> - try:
> - TmpValue, Size = ParseFieldValue(PcdValue)
> - except BadExpression, Value:
> - raise BadExpression("Type: %s, Value: %s, %s" % (self.PcdType,
> PcdValue, Value))
> + try:
> + TmpValue, Size = ParseFieldValue(PcdValue)
> + except BadExpression, Value:
> + raise BadExpression("Type: %s, Value: %s, %s" %
> + (self.PcdType, PcdValue, Value))
> if type(TmpValue) == type(''):
> try:
> TmpValue = int(TmpValue)
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/4] BaseTools: remove irrelevant code
2018-03-28 2:17 ` Zhu, Yonghong
2018-03-28 14:57 ` Carsey, Jaben
@ 2018-03-28 20:22 ` Carsey, Jaben
1 sibling, 0 replies; 11+ messages in thread
From: Carsey, Jaben @ 2018-03-28 20:22 UTC (permalink / raw)
To: Zhu, Yonghong, edk2-devel@lists.01.org; +Cc: Gao, Liming
Zhu,
I see what you meant. Please ignore this patch.
-Jaben
> -----Original Message-----
> From: Carsey, Jaben
> Sent: Wednesday, March 28, 2018 7:58 AM
> To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH v1 1/4] BaseTools: remove irrelevant code
>
> Zhu,
>
> Given that the statement ~1 line above "PcdValue = PcdValue.strip()" will
> raise exception with a PcdValue that is a list, I don't see how this code could
> execute.
>
> I agree that it is likely that we think we want this, but something is wrong
> with it and I assumed that the current behavior was the desired behavior. If I
> am incorrect, please feel free to disregard this patch.
>
> -Jaben
>
> > -----Original Message-----
> > From: Zhu, Yonghong
> > Sent: Tuesday, March 27, 2018 7:18 PM
> > To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org
> > Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong
> > <yonghong.zhu@intel.com>
> > Subject: RE: [PATCH v1 1/4] BaseTools: remove irrelevant code
> > Importance: High
> >
> > Hi Jaben,
> >
> > Current we support flexible PCD value format, example: UINT64 type PCD
> > can use {0x1, 2, 'A', UINT16(10), "B"} as its value, so the block can't be
> > removed since it maybe a list after SplitPcdValueString function.
> > https://bugzilla.tianocore.org/show_bug.cgi?id=541 is the feature request
> > for flexible Pcd value format.
> >
> > Best Regards,
> > Zhu Yonghong
> >
> >
> > -----Original Message-----
> > From: Carsey, Jaben
> > Sent: Wednesday, March 28, 2018 7:43 AM
> > To: edk2-devel@lists.01.org
> > Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong
> > <yonghong.zhu@intel.com>
> > Subject: [PATCH v1 1/4] BaseTools: remove irrelevant code
> >
> > Since PcdValue is a string, no need to test it's type() for string Also remove
> > the block used if it's a list (which is never is)
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
> > ---
> > BaseTools/Source/Python/Common/Expression.py | 55 ++------------------
> > 1 file changed, 5 insertions(+), 50 deletions(-)
> >
> > diff --git a/BaseTools/Source/Python/Common/Expression.py
> > b/BaseTools/Source/Python/Common/Expression.py
> > index 4f0f377f3788..b70f7da1233e 100644
> > --- a/BaseTools/Source/Python/Common/Expression.py
> > +++ b/BaseTools/Source/Python/Common/Expression.py
> > @@ -815,57 +815,12 @@ class ValueExpressionEx(ValueExpression):
> > except BadExpression, Value:
> > if self.PcdType in ['UINT8', 'UINT16', 'UINT32', 'UINT64', 'BOOLEAN']:
> > PcdValue = PcdValue.strip()
> > - if type(PcdValue) == type('') and PcdValue.startswith('{') and
> > PcdValue.endswith('}'):
> > + if PcdValue.startswith('{') and PcdValue.endswith('}'):
> > PcdValue = SplitPcdValueString(PcdValue[1:-1])
> > - if type(PcdValue) == type([]):
> > - TmpValue = 0
> > - Size = 0
> > - ValueType = ''
> > - for Item in PcdValue:
> > - Item = Item.strip()
> > - if Item.startswith('UINT8'):
> > - ItemSize = 1
> > - ValueType = 'UINT8'
> > - elif Item.startswith('UINT16'):
> > - ItemSize = 2
> > - ValueType = 'UINT16'
> > - elif Item.startswith('UINT32'):
> > - ItemSize = 4
> > - ValueType = 'UINT32'
> > - elif Item.startswith('UINT64'):
> > - ItemSize = 8
> > - ValueType = 'UINT64'
> > - elif Item.startswith('"') or Item.startswith("'") or
> > Item.startswith('L'):
> > - ItemSize = 0
> > - ValueType = 'VOID*'
> > - else:
> > - ItemSize = 0
> > - ValueType = 'UINT8'
> > - Item = ValueExpressionEx(Item, ValueType, self._Symb)(True)
> > -
> > - if ItemSize == 0:
> > - try:
> > - tmpValue = int(Item, 16) if Item.upper().startswith('0X')
> else
> > int(Item, 0)
> > - if tmpValue > 255:
> > - raise BadExpression("Byte array number %s should less
> > than 0xFF." % Item)
> > - except BadExpression, Value:
> > - raise BadExpression(Value)
> > - except ValueError:
> > - pass
> > - ItemValue, ItemSize = ParseFieldValue(Item)
> > - else:
> > - ItemValue = ParseFieldValue(Item)[0]
> > -
> > - if type(ItemValue) == type(''):
> > - ItemValue = int(ItemValue, 16) if ItemValue.startswith('0x')
> > else int(ItemValue)
> > -
> > - TmpValue = (ItemValue << (Size * 8)) | TmpValue
> > - Size = Size + ItemSize
> > - else:
> > - try:
> > - TmpValue, Size = ParseFieldValue(PcdValue)
> > - except BadExpression, Value:
> > - raise BadExpression("Type: %s, Value: %s, %s" %
> (self.PcdType,
> > PcdValue, Value))
> > + try:
> > + TmpValue, Size = ParseFieldValue(PcdValue)
> > + except BadExpression, Value:
> > + raise BadExpression("Type: %s, Value: %s, %s" %
> > + (self.PcdType, PcdValue, Value))
> > if type(TmpValue) == type(''):
> > try:
> > TmpValue = int(TmpValue)
> > --
> > 2.16.2.windows.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 2/4] BaseTools: expression can use single in instead of 3 API calls.
2018-03-27 23:42 ` [PATCH v1 2/4] BaseTools: expression can use single in instead of 3 API calls Jaben Carsey
@ 2018-03-29 2:55 ` Zhu, Yonghong
0 siblings, 0 replies; 11+ messages in thread
From: Zhu, Yonghong @ 2018-03-29 2:55 UTC (permalink / raw)
To: Carsey, Jaben, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
Best Regards,
Zhu Yonghong
-----Original Message-----
From: Carsey, Jaben
Sent: Wednesday, March 28, 2018 7:43 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [PATCH v1 2/4] BaseTools: expression can use single in instead of 3 API calls.
change 3 StartsWith() calls to a single 'in' operation.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/Python/Common/Expression.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/BaseTools/Source/Python/Common/Expression.py b/BaseTools/Source/Python/Common/Expression.py
index b70f7da1233e..3f2b43118553 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -948,7 +948,7 @@ class ValueExpressionEx(ValueExpression):
Item = '0x%x' % TmpValue if type(TmpValue) != type('') else TmpValue
if ItemSize == 0:
ItemValue, ItemSize = ParseFieldValue(Item)
- if not (Item.startswith('"') or Item.startswith('L') or Item.startswith('{')) and ItemSize > 1:
+ if Item[0] not in ['"','L','{'] and ItemSize > 1:
raise BadExpression("Byte array number %s should less than 0xFF." % Item)
else:
ItemValue = ParseFieldValue(Item)[0]
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 3/4] BaseTools: move regular expression compile out of function call.
2018-03-27 23:42 ` [PATCH v1 3/4] BaseTools: move regular expression compile out of function call Jaben Carsey
@ 2018-03-29 2:55 ` Zhu, Yonghong
0 siblings, 0 replies; 11+ messages in thread
From: Zhu, Yonghong @ 2018-03-29 2:55 UTC (permalink / raw)
To: Carsey, Jaben, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
Best Regards,
Zhu Yonghong
-----Original Message-----
From: Carsey, Jaben
Sent: Wednesday, March 28, 2018 7:43 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [PATCH v1 3/4] BaseTools: move regular expression compile out of function call.
move to the root of the file and dont recompile.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/Python/Common/Expression.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/BaseTools/Source/Python/Common/Expression.py b/BaseTools/Source/Python/Common/Expression.py
index 3f2b43118553..340c50ebe00f 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -41,6 +41,8 @@ ERR_EMPTY_EXPR = 'Empty expression is not allowed.'
ERR_IN_OPERAND = 'Macro after IN operator can only be: $(FAMILY), $(ARCH), $(TOOL_CHAIN_TAG) and $(TARGET).'
__ValidString = re.compile(r'[_a-zA-Z][_0-9a-zA-Z]*$')
+_ReLabel = re.compile('LABEL\((\w+)\)') _ReOffset =
+re.compile('OFFSET_OF\((\w+)\)')
## SplitString
# Split string to list according double quote @@ -853,13 +855,11 @@ class ValueExpressionEx(ValueExpression):
PcdValueList = SplitPcdValueString(PcdValue.strip()[1:-1])
LabelDict = {}
NewPcdValueList = []
- ReLabel = re.compile('LABEL\((\w+)\)')
- ReOffset = re.compile('OFFSET_OF\((\w+)\)')
LabelOffset = 0
for Index, Item in enumerate(PcdValueList):
# compute byte offset of every LABEL
- LabelList = ReLabel.findall(Item)
- Item = ReLabel.sub('', Item)
+ LabelList = _ReLabel.findall(Item)
+ Item = _ReLabel.sub('', Item)
Item = Item.strip()
if LabelList:
for Label in LabelList:
@@ -886,11 +886,11 @@ class ValueExpressionEx(ValueExpression):
# for LABEL parse
Item = Item.strip()
try:
- Item = ReLabel.sub('', Item)
+ Item = _ReLabel.sub('', Item)
except:
pass
try:
- OffsetList = ReOffset.findall(Item)
+ OffsetList = _ReOffset.findall(Item)
except:
pass
for Offset in OffsetList:
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 4/4] BaseTools: dont use enumerate when un-needed
2018-03-27 23:42 ` [PATCH v1 4/4] BaseTools: dont use enumerate when un-needed Jaben Carsey
@ 2018-03-29 2:55 ` Zhu, Yonghong
0 siblings, 0 replies; 11+ messages in thread
From: Zhu, Yonghong @ 2018-03-29 2:55 UTC (permalink / raw)
To: Carsey, Jaben, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
Best Regards,
Zhu Yonghong
-----Original Message-----
From: Carsey, Jaben
Sent: Wednesday, March 28, 2018 7:43 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [PATCH v1 4/4] BaseTools: dont use enumerate when un-needed
Since we only use the item from the list and not the numeric value, dont bother with enumerate()
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
---
BaseTools/Source/Python/Common/Expression.py | 4 ++--
BaseTools/Source/Python/Common/Misc.py | 2 +-
BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/BaseTools/Source/Python/Common/Expression.py b/BaseTools/Source/Python/Common/Expression.py
index 340c50ebe00f..f28d770aad94 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -856,7 +856,7 @@ class ValueExpressionEx(ValueExpression):
LabelDict = {}
NewPcdValueList = []
LabelOffset = 0
- for Index, Item in enumerate(PcdValueList):
+ for Item in PcdValueList:
# compute byte offset of every LABEL
LabelList = _ReLabel.findall(Item)
Item = _ReLabel.sub('', Item) @@ -882,7 +882,7 @@ class ValueExpressionEx(ValueExpression):
except:
LabelOffset = LabelOffset + 1
- for Index, Item in enumerate(PcdValueList):
+ for Item in PcdValueList:
# for LABEL parse
Item = Item.strip()
try:
diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py
index 7d44fdcf8ba7..8f479ace4cb1 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -76,7 +76,7 @@ def GetVariableOffset(mapfilepath, efifilepath, varnames):
def _parseForXcode(lines, efifilepath, varnames):
status = 0
ret = []
- for index, line in enumerate(lines):
+ for line in lines:
line = line.strip()
if status == 0 and line == "# Symbols:":
status = 1
diff --git a/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py b/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
index fdad5a44dc3d..0a701158e2c2 100644
--- a/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
+++ b/BaseTools/Source/Python/GenPatchPcdTable/GenPatchPcdTable.py
@@ -65,7 +65,7 @@ def parsePcdInfoFromMapFile(mapfilepath, efifilepath):
def _parseForXcode(lines, efifilepath):
status = 0
pcds = []
- for index, line in enumerate(lines):
+ for line in lines:
line = line.strip()
if status == 0 and line == "# Symbols:":
status = 1
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-03-29 2:49 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-27 23:42 [PATCH v1 0/4] BaseTools: remove code without affect Jaben Carsey
2018-03-27 23:42 ` [PATCH v1 1/4] BaseTools: remove irrelevant code Jaben Carsey
2018-03-28 2:17 ` Zhu, Yonghong
2018-03-28 14:57 ` Carsey, Jaben
2018-03-28 20:22 ` Carsey, Jaben
2018-03-27 23:42 ` [PATCH v1 2/4] BaseTools: expression can use single in instead of 3 API calls Jaben Carsey
2018-03-29 2:55 ` Zhu, Yonghong
2018-03-27 23:42 ` [PATCH v1 3/4] BaseTools: move regular expression compile out of function call Jaben Carsey
2018-03-29 2:55 ` Zhu, Yonghong
2018-03-27 23:42 ` [PATCH v1 4/4] BaseTools: dont use enumerate when un-needed Jaben Carsey
2018-03-29 2:55 ` Zhu, Yonghong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox