harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Regis <xu.re...@gmail.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 02:35:34 GMT
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

-- 
Best Regards,
Regis.

Mime
View raw message