You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Nathan Beyer <nb...@gmail.com> on 2009/07/14 16:48:25 UTC

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

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);
>
>

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

Posted by Oliver Deakin <ol...@googlemail.com>.
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


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

Posted by Regis <xu...@gmail.com>.
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.

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

Posted by Oliver Deakin <ol...@googlemail.com>.
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>
>          }
>
>

-- 
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


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

Posted by Regis <xu...@gmail.com>.
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>
          }


-- 
Best Regards,
Regis.

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

Posted by Oliver Deakin <ol...@googlemail.com>.
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);
>>
>>
>

-- 
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