Re: [edk2] [PATCH] BaseTools: Update Expression.py for string comparison and MACRO replace issue

2018-02-08 Thread Zhu, Yonghong 
Reviewed-by: Yonghong Zhu <yonghong@intel.com> 

Best Regards,
Zhu Yonghong


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Thursday, February 08, 2018 10:46 PM
To: edk2-devel@lists.01.org
Cc: Feng, YunhuaX <yunhuax.f...@intel.com>; Gao, Liming <liming@intel.com>
Subject: [edk2] [PATCH] BaseTools: Update Expression.py for string comparison 
and MACRO replace issue

From: Yunhua Feng <yunhuax.f...@intel.com>

1. Fix string comparison incorrect issue, we expected "ABC" is greater than 
"AAD" since the second char 'B' is greater than 'A'.
2. fix MACRO not replace issue.

Cc: Liming Gao <liming@intel.com>
Cc: Yonghong Zhu <yonghong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng <yunhuax.f...@intel.com>
---
 BaseTools/Source/Python/Common/Expression.py   | 41 ++
 BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
 .../Source/Python/Workspace/MetaFileParser.py  |  1 +
 3 files changed, 13 insertions(+), 31 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Expression.py 
b/BaseTools/Source/Python/Common/Expression.py
index 6a1103df2c..a19f35d991 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -155,23 +155,13 @@ class ValueExpression(object):
 
 @staticmethod
 def Eval(Operator, Oprand1, Oprand2 = None):
 WrnExp = None
 
-if Operator not in ["in", "not in"] and (type(Oprand1) == type('') or 
type(Oprand2) == type('')):
-if type(Oprand1) == type(''):
-if Oprand1[0] in ['"', "'"] or Oprand1.startswith('L"') or 
Oprand1.startswith("L'")or Oprand1.startswith('UINT'):
-Oprand1, Size = ParseFieldValue(Oprand1)
-else:
-Oprand1,Size = ParseFieldValue('"' + Oprand1 + '"')
-if type(Oprand2) == type(''):
-if Oprand2[0] in ['"', "'"] or Oprand2.startswith('L"') or 
Oprand2.startswith("L'") or Oprand2.startswith('UINT'):
-Oprand2, Size = ParseFieldValue(Oprand2)
-else:
-Oprand2, Size = ParseFieldValue('"' + Oprand2 + '"')
-if type(Oprand1) == type('') or type(Oprand2) == type(''):
-raise BadExpression(ERR_STRING_EXPR % Operator)
+if Operator not in ["==", "!=", ">=", "<=", ">", "<", "in", "not in"] 
and \
+(type(Oprand1) == type('') or type(Oprand2) == type('')):
+raise BadExpression(ERR_STRING_EXPR % Operator)
 if Operator in ['in', 'not in']:
 if type(Oprand1) != type(''):
 Oprand1 = IntToStr(Oprand1)
 if type(Oprand2) != type(''):
 Oprand2 = IntToStr(Oprand2) @@ -294,12 +284,10 @@ class 
ValueExpression(object):
 try:
 Token = self._GetToken()
 except BadExpression:
 pass
 if type(Token) == type('') and Token.startswith('{') and 
Token.endswith('}') and self._Idx >= self._Len:
-if len(Token) != len(self._Expr.replace(' ', '')):
-raise BadExpression
 return self._Expr
 
 self._Idx = 0
 self._Token = ''
 
@@ -457,19 +445,17 @@ class ValueExpression(object):
 Flag = 0
 for Index in range(len(self._Token)):
 if self._Token[Index] in ['"']:
 Flag += 1
 if Flag == 2 and self._Token.endswith('"'):
-self._Token = ParseFieldValue(self._Token)[0]
 return True
 if self._Token.startswith("'") or self._Token.startswith("L'"):
 Flag = 0
 for Index in range(len(self._Token)):
 if self._Token[Index] in ["'"]:
 Flag += 1
 if Flag == 2 and self._Token.endswith("'"):
-self._Token = ParseFieldValue(self._Token)[0]
 return True
 try:
 self._Token = int(self._Token, Radix)
 return True
 except ValueError:
@@ -620,28 +606,20 @@ class ValueExpression(object):
 if Expr.startswith('L"'):
 # Skip L
 self._Idx += 1
 UStr = self.__GetString()
 self._Token = 'L"' + UStr + '"'
-self._Token, Size = ParseFieldValue(self._Token)
 return self._Token
 elif Expr.startswith("L'"):
 # Skip L
 self._Idx += 1
 UStr = self.__GetString()
 self._Token = "

[edk2] [PATCH] BaseTools: Update Expression.py for string comparison and MACRO replace issue

2018-02-08 Thread Yonghong Zhu 
From: Yunhua Feng 

1. Fix string comparison incorrect issue, we expected "ABC" is greater than
"AAD" since the second char 'B' is greater than 'A'.
2. fix MACRO not replace issue.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/Common/Expression.py   | 41 ++
 BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
 .../Source/Python/Workspace/MetaFileParser.py  |  1 +
 3 files changed, 13 insertions(+), 31 deletions(-)

diff --git a/BaseTools/Source/Python/Common/Expression.py 
b/BaseTools/Source/Python/Common/Expression.py
index 6a1103df2c..a19f35d991 100644
--- a/BaseTools/Source/Python/Common/Expression.py
+++ b/BaseTools/Source/Python/Common/Expression.py
@@ -155,23 +155,13 @@ class ValueExpression(object):
 
 @staticmethod
 def Eval(Operator, Oprand1, Oprand2 = None):
 WrnExp = None
 
-if Operator not in ["in", "not in"] and (type(Oprand1) == type('') or 
type(Oprand2) == type('')):
-if type(Oprand1) == type(''):
-if Oprand1[0] in ['"', "'"] or Oprand1.startswith('L"') or 
Oprand1.startswith("L'")or Oprand1.startswith('UINT'):
-Oprand1, Size = ParseFieldValue(Oprand1)
-else:
-Oprand1,Size = ParseFieldValue('"' + Oprand1 + '"')
-if type(Oprand2) == type(''):
-if Oprand2[0] in ['"', "'"] or Oprand2.startswith('L"') or 
Oprand2.startswith("L'") or Oprand2.startswith('UINT'):
-Oprand2, Size = ParseFieldValue(Oprand2)
-else:
-Oprand2, Size = ParseFieldValue('"' + Oprand2 + '"')
-if type(Oprand1) == type('') or type(Oprand2) == type(''):
-raise BadExpression(ERR_STRING_EXPR % Operator)
+if Operator not in ["==", "!=", ">=", "<=", ">", "<", "in", "not in"] 
and \
+(type(Oprand1) == type('') or type(Oprand2) == type('')):
+raise BadExpression(ERR_STRING_EXPR % Operator)
 if Operator in ['in', 'not in']:
 if type(Oprand1) != type(''):
 Oprand1 = IntToStr(Oprand1)
 if type(Oprand2) != type(''):
 Oprand2 = IntToStr(Oprand2)
@@ -294,12 +284,10 @@ class ValueExpression(object):
 try:
 Token = self._GetToken()
 except BadExpression:
 pass
 if type(Token) == type('') and Token.startswith('{') and 
Token.endswith('}') and self._Idx >= self._Len:
-if len(Token) != len(self._Expr.replace(' ', '')):
-raise BadExpression
 return self._Expr
 
 self._Idx = 0
 self._Token = ''
 
@@ -457,19 +445,17 @@ class ValueExpression(object):
 Flag = 0
 for Index in range(len(self._Token)):
 if self._Token[Index] in ['"']:
 Flag += 1
 if Flag == 2 and self._Token.endswith('"'):
-self._Token = ParseFieldValue(self._Token)[0]
 return True
 if self._Token.startswith("'") or self._Token.startswith("L'"):
 Flag = 0
 for Index in range(len(self._Token)):
 if self._Token[Index] in ["'"]:
 Flag += 1
 if Flag == 2 and self._Token.endswith("'"):
-self._Token = ParseFieldValue(self._Token)[0]
 return True
 try:
 self._Token = int(self._Token, Radix)
 return True
 except ValueError:
@@ -620,28 +606,20 @@ class ValueExpression(object):
 if Expr.startswith('L"'):
 # Skip L
 self._Idx += 1
 UStr = self.__GetString()
 self._Token = 'L"' + UStr + '"'
-self._Token, Size = ParseFieldValue(self._Token)
 return self._Token
 elif Expr.startswith("L'"):
 # Skip L
 self._Idx += 1
 UStr = self.__GetString()
 self._Token = "L'" + UStr + "'"
-self._Token, Size = ParseFieldValue(self._Token)
-return self._Token
-elif Expr.startswith('"'):
-UStr = self.__GetString()
-self._Token = '"' + UStr + '"'
-self._Token, Size = ParseFieldValue(self._Token)
 return self._Token
 elif Expr.startswith("'"):
 UStr = self.__GetString()
 self._Token = "'" + UStr + "'"
-self._Token, Size = ParseFieldValue(self._Token)
 return self._Token
 elif Expr.startswith('UINT'):
 Re = re.compile('(?:UINT8|UINT16|UINT32|UINT64)\((.+)\)')
 try:
 RetValue = Re.search(Expr).group(1)
@@ -749,11 +727,11 @@ class ValueExpressionEx(ValueExpression):