You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/12/04 22:21:11 UTC
svn commit: r1417194 - in /tomcat/trunk/java/org/apache/tomcat/util/buf:
ByteChunk.java CharChunk.java
Author: markt
Date: Tue Dec 4 21:21:11 2012
New Revision: 1417194
URL: http://svn.apache.org/viewvc?rev=1417194&view=rev
Log:
Replicate the equals() and hashCode() strategy from MessageBytes to silence some FindBugs warnings
Modified:
tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.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=1417194&r1=1417193&r2=1417194&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java Tue Dec 4 21:21:11 2012
@@ -101,6 +101,10 @@ public final class ByteChunk implements
*/
public static final Charset DEFAULT_CHARSET = B2CConverter.ISO_8859_1;
+ private int hashCode=0;
+ // did we compute the hashcode ?
+ private boolean hasHashCode = false;
+
// byte[]
private byte[] buff;
@@ -141,6 +145,7 @@ public final class ByteChunk implements
start=0;
end=0;
isSet=false;
+ hasHashCode = false;
}
public void reset() {
@@ -157,6 +162,7 @@ public final class ByteChunk implements
start=0;
end=0;
isSet=true;
+ hasHashCode = false;
}
/**
@@ -171,6 +177,7 @@ public final class ByteChunk implements
start = off;
end = start+ len;
isSet=true;
+ hasHashCode = false;
}
public void setCharset(Charset charset) {
@@ -488,6 +495,14 @@ public final class ByteChunk implements
// -------------------- equals --------------------
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof ByteChunk) {
+ return equals((ByteChunk) obj);
+ }
+ return false;
+ }
+
/**
* Compares the message bytes to the specified String object.
* @param s the String to compare
@@ -626,6 +641,19 @@ public final class ByteChunk implements
// -------------------- Hash code --------------------
+ @Override
+ public int hashCode() {
+ if (hasHashCode) {
+ return hashCode;
+ }
+ int code = 0;
+
+ code = hash();
+ hashCode = code;
+ hasHashCode = true;
+ return code;
+ }
+
// normal hash.
public int hash() {
return hashBytes( buff, start, end-start);
Modified: tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.java?rev=1417194&r1=1417193&r2=1417194&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.java Tue Dec 4 21:21:11 2012
@@ -57,6 +57,11 @@ public final class CharChunk implements
}
// --------------------
+
+ private int hashCode = 0;
+ // did we compute the hashcode ?
+ private boolean hasHashCode = false;
+
// char[]
private char buff[];
@@ -99,6 +104,7 @@ public final class CharChunk implements
public void recycle() {
// buff=null;
isSet=false; // XXX
+ hasHashCode = false;
start=0;
end=0;
}
@@ -113,6 +119,7 @@ public final class CharChunk implements
start=0;
end=0;
isSet=true;
+ hasHashCode = false;
}
@@ -125,6 +132,7 @@ public final class CharChunk implements
start=off;
end=start + len;
isSet=true;
+ hasHashCode = false;
}
/** Maximum amount of data in this buffer.
@@ -455,6 +463,14 @@ public final class CharChunk implements
// -------------------- equals --------------------
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof CharChunk) {
+ return equals((CharChunk) obj);
+ }
+ return false;
+ }
+
/**
* Compares the message bytes to the specified String object.
* @param s the String to compare
@@ -578,6 +594,19 @@ public final class CharChunk implements
// -------------------- Hash code --------------------
+ @Override
+ public int hashCode() {
+ if (hasHashCode) {
+ return hashCode;
+ }
+ int code = 0;
+
+ code = hash();
+ hashCode = code;
+ hasHashCode = true;
+ return code;
+ }
+
// normal hash.
public int hash() {
int code=0;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1417194 - in /tomcat/trunk/java/org/apache/tomcat/util/buf: ByteChunk.java CharChunk.java
Posted by Mark Thomas <ma...@apache.org>.
Christopher Schultz <ch...@christopherschultz.net> wrote:
>Mark,
>> // -------------------- Hash code --------------------
>>
>> + @Override
>> + public int hashCode() {
>> + if (hasHashCode) {
>> + return hashCode;
>> + }
>> + int code = 0;
>> +
>> + code = hash();
>> + hashCode = code;
>> + hasHashCode = true;
>> + return code;
>> + }
>
>Any particular reason for the dead store of 0 to 'code' local?
>
>In fact, there seems to be much more code in there than necessary. Why
>not:
The code was copied directly from MessageBytes (as per the original commit log). My primary concern for this commit was consistency rather than improving the code. Any changes need to be made to all three classes.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1417194 - in /tomcat/trunk/java/org/apache/tomcat/util/buf:
ByteChunk.java CharChunk.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/12/8 Caldarale, Charles R <Ch...@unisys.com>:
>> From: Christopher Schultz [mailto:chris@christopherschultz.net]
>> Subject: Re: svn commit: r1417194 - in /tomcat/trunk/java/org/apache/tomcat/util/buf: ByteChunk.java CharChunk.java
>
>> On 12/4/12 4:21 PM, markt@apache.org wrote:
>> > + private int hashCode=0;
>> > + // did we compute the hashcode ?
>> > + private boolean hasHashCode = false;
>
>> Should hashCode and hasHashCode be volatile?
>
> Yes, to insure program order is followed; otherwise there's no guarantee that hashCode and hasHashCode are written in the required sequence.
1) I would prefer a single volatile field rather than dealing with two
volatile ones.
E.g. make sure that hash() never returns 0 and use that as a flag.
If it is a single field, there is no need to make it volatile,
likewise the buff/start/end fields are not volatile.
2) I see no need for the public "hash()" method here. The MessageBytes
class can be changed to call hashCode().
>> In fact, there seems to be much more code in there than necessary. Why not:
>
>> > + public int hashCode() {
>> > + if (hasHashCode) {
>> > + return hashCode;
>> > + }
>> > + hashCode = hash();
>> > + hasHashCode = true;
>> > + return hashCode;
>
> Or:
>
> public int hashCode() {
> if (!hasHashCode) {
> hashCode = hash();
> hasHashCode = true;
> }
> return hashCode;
> }
>
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: r1417194 - in
/tomcat/trunk/java/org/apache/tomcat/util/buf: ByteChunk.java CharChunk.java
Posted by "Caldarale, Charles R" <Ch...@unisys.com>.
> From: Christopher Schultz [mailto:chris@christopherschultz.net]
> Subject: Re: svn commit: r1417194 - in /tomcat/trunk/java/org/apache/tomcat/util/buf: ByteChunk.java CharChunk.java
> On 12/4/12 4:21 PM, markt@apache.org wrote:
> > + private int hashCode=0;
> > + // did we compute the hashcode ?
> > + private boolean hasHashCode = false;
> Should hashCode and hasHashCode be volatile?
Yes, to insure program order is followed; otherwise there's no guarantee that hashCode and hasHashCode are written in the required sequence.
> In fact, there seems to be much more code in there than necessary. Why not:
> > + public int hashCode() {
> > + if (hasHashCode) {
> > + return hashCode;
> > + }
> > + hashCode = hash();
> > + hasHashCode = true;
> > + return hashCode;
Or:
public int hashCode() {
if (!hasHashCode) {
hashCode = hash();
hasHashCode = true;
}
return hashCode;
}
- Chuck
THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1417194 - in /tomcat/trunk/java/org/apache/tomcat/util/buf:
ByteChunk.java CharChunk.java
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 12/4/12 4:21 PM, markt@apache.org wrote:
> + private int hashCode=0;
> + // did we compute the hashcode ?
> + private boolean hasHashCode = false;
Should hashCode and hasHashCode be volatile?
> // -------------------- Hash code --------------------
>
> + @Override
> + public int hashCode() {
> + if (hasHashCode) {
> + return hashCode;
> + }
> + int code = 0;
> +
> + code = hash();
> + hashCode = code;
> + hasHashCode = true;
> + return code;
> + }
Any particular reason for the dead store of 0 to 'code' local?
In fact, there seems to be much more code in there than necessary. Why not:
> + public int hashCode() {
> + if (hasHashCode) {
> + return hashCode;
> + }
> + hashCode = hash();
> + hasHashCode = true;
> + return hashCode;
-chris