dev@glassfish.java.net

Re: [Fwd: FindBugs warnings in glassfish-v2-b26 (was Re: V2 Milestone 3 Completion Criteria)]

From: Lloyd L Chambers <Lloyd.Chambers_at_Sun.COM>
Date: Fri, 08 Dec 2006 13:51:25 -0800

Where do I find the FindBugs output on the GlassFish site?


Lloyd

On Dec 8, 2006, at 8:43 AM, Geoff Halliwell wrote:

> [Cross posting for a wider audience...]
>
> From: Bill Pugh <pugh_at_cs.umd.edu>
> Date: December 8, 2006 7:00:07 AM PST
> To: quality_at_glassfish.dev.java.net
> Subject: FindBugs warnings in glassfish-v2-b26 (was Re: V2
> Milestone 3 Completion Criteria)
> Reply-To: quality_at_glassfish.dev.java.net
>
>
> 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.GeneratedMonitoringMBean
> Impl
>
> 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
>
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net