dev@glassfish.java.net

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

From: Jan Luehe <Jan.Luehe_at_Sun.COM>
Date: Fri, 08 Dec 2006 14:28:33 -0800

Lloyd L Chambers wrote On 12/08/06 13:51,:

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


There's a link ("FindBugs Reports: Full
<https://glassfish.dev.java.net/quality/2006-09-28-fullreport.html>,
Detailed
<https://glassfish.dev.java.net/quality/2006-09-28-fancyreport.html>")
off of

  http://wiki.java.net/bin/view/Projects/GlassFishQuality

but it's not very useful because the report has not been updated
since 09/28.


Jan

>
>
> 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 <mailto:pugh_at_cs.umd.edu>>
>> *Date: *December 8, 2006 7:00:07 AM PST
>> *To: *quality_at_glassfish.dev.java.net
>> <mailto: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
>> <mailto: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.GeneratedMonitoringMBeanImpl
>>
>> 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
>> <mailto:dev-unsubscribe_at_glassfish.dev.java.net>
>> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>> <mailto:dev-help_at_glassfish.dev.java.net>
>
>