harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Deakin <oliver.dea...@googlemail.com>
Subject Re: svn commit: r793587 - in /harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser: FilterParserTokenManager.java LdapUrlParserTokenManager.java
Date Thu, 16 Jul 2009 12:51:03 GMT
Thanks Regis - commited at r794655.

Regards,
Oliver

Regis wrote:
> Oliver Deakin wrote:
>> Thanks for the help Regis - I applied the patch and regenerated the 
>> java classes using the latest javacc but still hit a problem on z/OS. 
>> I figured out that we also need to add "/u0085" to the CHAR 
>> definition in url.g and filter.g [1]. This fixes the test failures on 
>> z/OS for me, and there are no new failures on Windows x86 either.
>>
>> If you agree that the patch below is right then I will apply it along 
>> with the newly generated java classes.
>>
>> Regards,
>> Oliver
>>
>> [1]
>> Index: filter.g
>> ===================================================================
>> --- filter.g    (revision 793921)
>> +++ filter.g    (working copy)
>> @@ -214,7 +214,7 @@
>> |
>> <SEMI: ";">
>> |
>> -<CHAR : ~["*", "\\", "(", ")", "\n"] >
>> +<CHAR : ~["*", "\\", "(", ")", "\n", "\u0085"] >
>> }
>>
>> String option():
>> @@ -317,7 +317,7 @@
>>                 }
>>             }
>>
>> -            parse() "\n" test() | LOOKAHEAD(2) "\n" | "\n" <EOF> | 
>> <EOF>
>> +            parse() ("\n" | "\u0085") test() | LOOKAHEAD(2) ("\n" | 
>> "\u0085") | ("\n" | "\u0085") <EOF> | <EOF>
>>         }
>> // FIXME: get string representation of AttributeValue, then use 
>> Rdn.unescapeValue(String) to get value
>> String value():
>> Index: url.g
>> ===================================================================
>> --- url.g    (revision 793921)
>> +++ url.g    (working copy)
>> @@ -181,7 +181,7 @@
>> |
>> <ZERO : "0">
>> |
>> -<CHAR : ~["/", "?", "\n"] >
>> +<CHAR : ~["/", "?", "\n", "\u0085"] >
>>
>> }
>>
>> @@ -394,5 +394,5 @@
>> void test():
>>         {}
>>         {
>> -            parseURL() "\n" test() | LOOKAHEAD(2) "\n" | "\n" <EOF> 
>> | <EOF>
>> +            parseURL() ("\n" | "\u0085") test() | LOOKAHEAD(2) ("\n" 
>> | "\u0085") | ("\n" | "\u0085") <EOF> | <EOF>
>>         }
>>
>>
>>
>> Regis wrote:
>>> Oliver Deakin wrote:
>>>> Yes, that's right actually - Ive been looking at the grammar files 
>>>> but can't figure out the right place to make the change. Does 
>>>> anyone have a good enough knowledge of the javacc tool to figure 
>>>> out the correct change?
>>>>
>>>> Regards,
>>>> Oliver
>>>>
>>>> Nathan Beyer wrote:
>>>>> Is this class generated code? Should the change be made to a BNF 
>>>>> file instead?
>>>>>
>>>>> Sent from my iPhone
>>>>>
>>>>> On Jul 13, 2009, at 8:54 AM, odeakin@apache.org wrote:
>>>>>
>>>>>> Author: odeakin
>>>>>> Date: Mon Jul 13 13:54:57 2009
>>>>>> New Revision: 793587
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=793587&view=rev
>>>>>> Log:
>>>>>> Handle the z/OS NEL character correctly when parsing tokens.
>>>>>>
>>>>>> Modified:
>>>>>>    
>>>>>> harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/FilterParserTokenManager.java

>>>>>>
>>>>>>    
>>>>>> harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/LdapUrlParserTokenManager.java

>>>>>>
>>>>>>
>>>>>> Modified: 
>>>>>> harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/FilterParserTokenManager.java

>>>>>>
>>>>>> URL: 
>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/FilterParserTokenManager.java?rev=793587&r1=793586&r2=793587&view=diff

>>>>>>
>>>>>> ==============================================================================

>>>>>>
>>>>>> --- 
>>>>>> harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/FilterParserTokenManager.java

>>>>>> (original)
>>>>>> +++ 
>>>>>> harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/FilterParserTokenManager.java

>>>>>> Mon Jul 13 13:54:57 2009
>>>>>> @@ -63,6 +63,7 @@
>>>>>>    switch(curChar)
>>>>>>    {
>>>>>>       case 10:
>>>>>> +      case 133: // NEL character for z/OS new lines
>>>>>>          return jjStopAtPos(0, 24);
>>>>>>       case 33:
>>>>>>          return jjStopAtPos(0, 10);
>>>>>>
>>>>>> Modified: 
>>>>>> harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/LdapUrlParserTokenManager.java

>>>>>>
>>>>>> URL: 
>>>>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/LdapUrlParserTokenManager.java?rev=793587&r1=793586&r2=793587&view=diff

>>>>>>
>>>>>> ==============================================================================

>>>>>>
>>>>>> --- 
>>>>>> harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/LdapUrlParserTokenManager.java

>>>>>> (original)
>>>>>> +++ 
>>>>>> harmony/enhanced/classlib/trunk/modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/LdapUrlParserTokenManager.java

>>>>>> Mon Jul 13 13:54:57 2009
>>>>>> @@ -61,6 +61,7 @@
>>>>>>    switch(curChar)
>>>>>>    {
>>>>>>       case 10:
>>>>>> +      case 133: // NEL character for z/OS new lines
>>>>>>          return jjStopAtPos(0, 17);
>>>>>>       case 33:
>>>>>>          return jjStopAtPos(0, 10);
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>> [1] is a quick fix for this, I don't have z/OS machine, so can't 
>>> test if it works.
>>>
>>> Actually this part of code is only by method test(), a convenient 
>>> way to verify whether parsers work correctly. It doesn't break 
>>> parsing functions if removing test() method and "new lines" should 
>>> be not a problem, but some test cases will be broken. I'll try to 
>>> remove test() method and rewrite test cases depend on it.
>>>
>>> [1]
>>>
>>> Index: 
>>> modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/filter.g

>>>
>>> =====================================================================
>>> --- 
>>> modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/filter.g

>>>
>>> +++ 
>>> modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/filter.g

>>>
>>> @@ -317,7 +317,7 @@ void test():
>>>                  }
>>>              }
>>>
>>> -            parse() "\n" test() | LOOKAHEAD(2) "\n" | "\n" <EOF> | 
>>> <EOF>
>>> +            parse() ("\n" | "\u0085") test() | LOOKAHEAD(2) ("\n" | 
>>> "\u0085") | ("\n" | "\u0085") <EOF> | <EOF>
>>>          }
>>>  // FIXME: get string representation of AttributeValue, then use 
>>> Rdn.unescapeValue(String) to get value
>>>  String value():
>>> Index: 
>>> modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/url.g

>>>
>>> =====================================================================
>>> --- 
>>> modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/url.g

>>>
>>> +++ 
>>> modules/jndi/src/main/java/org/apache/harmony/jndi/provider/ldap/parser/url.g

>>>
>>> @@ -394,5 +394,5 @@ String oid():
>>>  void test():
>>>          {}
>>>          {
>>> -            parseURL() "\n" test() | LOOKAHEAD(2) "\n" | "\n" <EOF> 
>>> | <EOF>
>>> +            parseURL() ("\n" | "\u0085") test() | LOOKAHEAD(2) 
>>> ("\n" | "\u0085") | ("\n" | "\u0085") <EOF> | <EOF>
>>>          }
>>>
>>>
>>
>
> The patch looks good for me. +1
>

-- 
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


Mime
View raw message