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 14:29:11 -0800

Bummer. Can't a text file just be automatically 'cvs commit'ed?

Lloyd

On Dec 8, 2006, at 2:24 PM, Geoff Halliwell wrote:

> Currently, it's only available on a regular basis internally.
>
> We have plans to publish them externally, but other things
> have taken a high priority.
>
> Geoff
>
> Lloyd L Chambers wrote On 12/08/06 13:51,:
>> 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 <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.GeneratedMonitoringMBe
>>> anImpl
>>>
>>> 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>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: dev-help_at_glassfish.dev.java.net
>