You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2010/05/17 18:28:45 UTC
svn commit: r945231 -
/tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
Author: kkolinko
Date: Mon May 17 16:28:44 2010
New Revision: 945231
URL: http://svn.apache.org/viewvc?rev=945231&view=rev
Log:
Fix a bug in ByteChunk.indexOf(String, ...)
The problem is that the method could not find a string which length is 1,
as the only successful exit from the method was from inside the loop that
checks the second and subsequent characters.
I added the testcase for this in r945230.
In the test the bc.indexOf("o", 0, 1, 5) call returned -1, instead of 7.
Modified:
tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
Modified: tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java?rev=945231&r1=945230&r2=945231&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java Mon May 17 16:28:44 2010
@@ -685,15 +685,16 @@ public final class ByteChunk implements
// Look for first char
int srcEnd = srcOff + srcLen;
+ mainLoop:
for( int i=myOff+start; i <= (end - srcLen); i++ ) {
if( buff[i] != first ) continue;
// found first char, now look for a match
int myPos=i+1;
for( int srcPos=srcOff + 1; srcPos< srcEnd; ) {
if( buff[myPos++] != src.charAt( srcPos++ ))
- break;
- if( srcPos==srcEnd ) return i-start; // found it
+ continue mainLoop;
}
+ return i-start; // found it
}
return -1;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r945231 - /tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2010/5/17 Mark Thomas <ma...@apache.org>:
> On 17/05/2010 18:08, Mark Thomas wrote:
>> On 17/05/2010 17:50, Konstantin Kolinko wrote:
>>> I am a bit shivering looking at ([a++] != [b++]),
>>> but I did minimal changes to fix the issue.
>>
>> Understood. If I get a chance, I'll look to see if I can find an equally
>> good alternative.
>
> How about (diff to original file)
>
It fails for a reason. There should be if (srcLen == 1).
BTW, my patch removes if( srcPos==srcEnd ) check inside the loop,
so it may run a bit faster. :] Though that would be hard to measure.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r945231 - /tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
Posted by Mark Thomas <ma...@apache.org>.
On 17/05/2010 18:08, Mark Thomas wrote:
> On 17/05/2010 17:50, Konstantin Kolinko wrote:
>> I am a bit shivering looking at ([a++] != [b++]),
>> but I did minimal changes to fix the issue.
>
> Understood. If I get a chance, I'll look to see if I can find an equally
> good alternative.
How about (diff to original file)
Index: java/org/apache/tomcat/util/buf/ByteChunk.java
===================================================================
--- java/org/apache/tomcat/util/buf/ByteChunk.java (revision 945095)
+++ java/org/apache/tomcat/util/buf/ByteChunk.java (working copy)
@@ -688,6 +688,9 @@
for( int i=myOff+start; i <= (end - srcLen); i++ ) {
if( buff[i] != first ) continue;
// found first char, now look for a match
+ if (src.length() == 1) {
+ return i-start;
+ }
int myPos=i+1;
for( int srcPos=srcOff + 1; srcPos< srcEnd; ) {
if( buff[myPos++] != src.charAt( srcPos++ ))
Your test:
assertEquals(0, bc.indexOf("wo", 0, 1, 0));
fails but given "wo".length != 1 then I'm not sure that is an issue.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r945231 - /tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
Posted by Mark Thomas <ma...@apache.org>.
On 17/05/2010 17:50, Konstantin Kolinko wrote:
> 2010/5/17 Mark Thomas <ma...@apache.org>:
>>
>> That's pretty ugly. Is there really no other way?
>>
>
> You mean, the named loop is ugly?
>
> Or the old code?
It was the named loop.
> I am a bit shivering looking at ([a++] != [b++]),
> but I did minimal changes to fix the issue.
Understood. If I get a chance, I'll look to see if I can find an equally
good alternative.
> The code runs frequently, so I think/hope that somebody
> has tried to optimize it for performance.
It certainly looks like it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r945231 - /tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2010/5/17 Mark Thomas <ma...@apache.org>:
>
> That's pretty ugly. Is there really no other way?
>
You mean, the named loop is ugly?
Or the old code?
I am a bit shivering looking at ([a++] != [b++]),
but I did minimal changes to fix the issue.
The code runs frequently, so I think/hope that somebody
has tried to optimize it for performance.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r945231 - /tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
Posted by Mark Thomas <ma...@apache.org>.
On 17/05/2010 17:28, kkolinko@apache.org wrote:
> Author: kkolinko
> Date: Mon May 17 16:28:44 2010
> New Revision: 945231
>
> URL: http://svn.apache.org/viewvc?rev=945231&view=rev
> Log:
> Fix a bug in ByteChunk.indexOf(String, ...)
>
> The problem is that the method could not find a string which length is 1,
> as the only successful exit from the method was from inside the loop that
> checks the second and subsequent characters.
>
> I added the testcase for this in r945230.
> In the test the bc.indexOf("o", 0, 1, 5) call returned -1, instead of 7.
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java?rev=945231&r1=945230&r2=945231&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java Mon May 17 16:28:44 2010
> @@ -685,15 +685,16 @@ public final class ByteChunk implements
> // Look for first char
> int srcEnd = srcOff + srcLen;
>
> + mainLoop:
> for( int i=myOff+start; i <= (end - srcLen); i++ ) {
> if( buff[i] != first ) continue;
> // found first char, now look for a match
> int myPos=i+1;
> for( int srcPos=srcOff + 1; srcPos< srcEnd; ) {
> if( buff[myPos++] != src.charAt( srcPos++ ))
> - break;
> - if( srcPos==srcEnd ) return i-start; // found it
> + continue mainLoop;
> }
> + return i-start; // found it
> }
> return -1;
> }
That's pretty ugly. Is there really no other way?
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org