On Oct 27, 2006, at 5:08 PM, Geoff Halliwell wrote:
> Posting, as I'm not sure everyone has seen the M3 Completion Criteria.
>
> http://www.glassfishwiki.org/gfwiki/Wiki.jsp?
> page=Milestone3ExitCriteria
>
> Note: Critical FindBugs issues need to be resolved by 12/11.
>
> Geoff
I haven't really been in the loop, but there are a lot of
critical FindBugs issues still in glassfish-v2-b26.
FindBugs 1.1.2rc2 generates 84 high priority correctness warnings
and 367 medium priority correctness warnings (these are just warnings
on files that occur in the source distribution).
We've done a lot of tuning since FindBugs 1.0, and with the current
version
of FindBugs, I've generally found that at least 2/3 of the medium/
high priority
correctness warnings translate into things you _really_ want to fix.
I'd be happy to chat or email with anyone interested in addressing
these issues;
I've love for Glassfish to be a FindBugs success story and ship
fairly clean,
but at the moment it looks fairly nasty.
Some samples:
4 impossible casts:
com.sun.enterprise.admin.monitor.registry.spi.GeneratedMonitoringMBeanIm
pl
public String[] listAttributes(){
return (String[]) attributeMap.values().toArray(); // BUG:
toArray returns an Object[] which
// can't
be downcast
}
3 calls to methods of containers with wrong types.
com.sun.appserv.management.util.jmx.NotificationEmitterSupport:
decrementListenerCountForType( final String type )
{
synchronized( mListenerTypeCounts )
{
final Integer count = mListenerTypeCounts.get( type );
if ( count == null )
{
throw new IllegalArgumentException( type );
}
final int oldValue = count.intValue();
if ( oldValue == 1 )
{
mListenerTypeCounts.remove( count ); // BUG: key of the
Map is a String
// This
remove will be a no-op, and count
// will
never go to zero/null
// should be .remove( type)
}
else
{
mListenerTypeCounts.put( type, new Integer( oldValue -
1 ) );
}
}
}
1 bad comparison of signed byte:
org.apache.jasper.xmlparser.ASCIIReader:
public int read(char ch[], int offset, int length) throws IOException {
if (length > fBuffer.length) {
length = fBuffer.length;
}
int count = fInputStream.read(fBuffer, 0, length);
for (int i = 0; i < count; i++) {
int b0 = fBuffer[i];
if (b0 > 0x80) { // BUG: b0 is in the range -128 .. 127
throw new IOException(Localizer.getMessage
("jsp.error.xml.invalidASCII",
Integer.toString(b0)));
}
ch[offset + i] = (char)b0;
}
return count;
}
9 ignored return values:
com.sun.appserv.management.alert.UnitTest
private void unitTestFailed( String propertyName, String expectedValue,
String realValue )
{
System.out.println( "UNIT TEST FAILED FOR ALERTS : " +
"\nPropertyName -> " + propertyName +
"\nexpectedValue -> " + expectedValue +
"\nrealValue -> " + realValue );
new RuntimeException( "Unit Test Failed For Alerts.." ); //
BUG, not thrown
}
9 dead stores due to switch fall throughs:
com.sun.enterprise.deployment.MethodDescriptor:
// Convert arrays from [[[I form to int[][][] form.
public static String fixParamClassName(String param)
{
if ( param.charAt(0) == '[' ) { // an array
int dimensions = param.lastIndexOf('[') + 1;
char code = param.charAt(dimensions);
String newparam=null;
switch (code) { // LOTS OF BUGS: no break statements
case 'B':
newparam = "byte";
case 'C':
newparam = "char";
case 'D':
newparam = "double";
case 'F':
newparam = "float";
case 'I':
newparam = "int";
case 'J':
newparam = "long";
case 'S':
newparam = "short";
case 'Z':
newparam = "boolean";
case 'L':
newparam = param.substring(dimensions+1);
}
for ( int j=0; j<dimensions; j++ )
newparam += "[]";
return newparam;
}
...
11 self assignments of a field in a constructor:
com.sun.appserv.management.j2ee.statistics.BoundedRangeStatisticImpl:
public
BoundedRangeStatisticImpl(
final String name,
final String description,
final String unit,
final long startTime,
final long lastSampleTime,
final long low,
final long current,
final long high,
final long lowerBound,
final long upperBound )
{
super( name, description, unit, startTime, lastSampleTime, low,
current, high );
if ( lowerBound > upperBound )
{
throw new IllegalArgumentException();
}
LowerBound = lowerBound;
UpperBound = UpperBound; // THIS IS A BUG; the parameter is upperBound
}
3 uncallable method of an anonymous inner class
javax.el.BeanELResolver:
private static final Map<Class, BeanProperties> properties =
Collections.synchronizedMap(new LinkedHashMap
(CACHE_INIT_SIZE, 0.75f, true){
public BeanProperties put(Class key, BeanProperties
value) { // BUG: should be put(Object, Object)
SoftReference<BeanProperties> ref = new
SoftReference(value);
SoftReference<BeanProperties> prev =
(SoftReference<BeanProperties>)super.put(key,
ref);
if (prev != null) return prev.get();
return null;
}
public BeanProperties get(Class key) { // THIS IS A
BUG: Should be get(Object)
SoftReference<BeanProperties> ref =
(SoftReference<BeanProperties>)super.get(key);
if (ref != null && ref.get() == null) {
remove(key);
}
return ref != null ? ref.get() : null;
}
protected boolean removeEldestEntry(Map.Entry eldest) {
return size() > CACHE_MAX_SIZE;
}
});
Actually, a better way to fix this is to make the anonymous inner
class extend
LinkedHashMap<Class, BeanProperties>, so that the appropriate bridge
methods
will be generated.
Plus 190 null pointer warnings.
Bill Pugh