Findbugs and comparing

Question

I recently started using the findbugs static analysis tool in a java build I was doing. The first report came back with loads of High Priority warnings. Being the obsessive type of person, I was ready to go knock them all out. However, I must be missing something. I get most of the warnings when comparing things. Such as the following code:

   public void setSpacesPerLevel(int value)
   {
      if( value >= 0)
      {
         spacesPerLevel = value;
      }
      else
      {
         spacesPerLevel = 0;
      }
   }

produces a high priority warning at the if statement that reads.

File: Indenter.java, Line: 60, Type: BIT_AND_ZZ, Priority: High, Category: CORRECTNESS Check to see if ((...) & 0) == 0 in sample.Indenter.setSpacesPerLevel(int)

I am comparing an int to an int, seems like a common thing. I get quite a few of that type of error with similar simple comparisons.

I have alot of other high priority warnings on what appears to be simple code blocks. Am I missing something here? I realize that static analysis can produce false positives, but the errors I am seeing seem too trivial of a case to be a false positive.

This one has me scratching my head as well.

    for(int spaces = 0;spaces < spacesPerLevel;spaces++)
    {
       result = result.concat(" ");
    }

Which gives the following findbugs warning:

File: Indenter.java, Line: 160, Type: IL_INFINITE_LOOP, Priority: High, Category: CORRECTNESS

There is an apparent infinite loop in sample.Indenter.indent()

This loop doesn't seem to have a way to terminate (other than by perhaps throwing an exception).

Any ideas?

So basically I have a handful of files and 50-60 high priority warnings similar to the ones above. I am using findbugs 1.3.9 and calling it from the findbugs ant task

UPDATE: I have this build being executed by a hudson server and had the code being instrumented by Clover for code coverage. When I turned that off, all of my high priority warnings disappeared. That makes sense now. Thanks for the feedback.

Answer 1

Are you running Findbugs thru Eclipse plugin, ant or gui? is it possible that the code hasn't recompiled since you ran it (before making changes)?

if setSpacesPerLevel isn't too long, post the output of

javap -v TheClassThatContainssetSpacerPerLevel

As for the second bug, you'd have to show the whole loop before one could say if it was a problem.

Answer 2

UPDATE: I have this build being executed by a hudson server and had the code being instrumented by Clover for code coverage. When I turned that off, all of my high priority warnings disappeared. That makes sense now. Thanks for the feedback.

Answer 3

A side note:

for(int spaces = 0;spaces < spacesPerLevel;spaces++)
{
   result = result.concat(" ");
}

If result is a java.lang.String, this may be inefficient, as you do the following steps for each space character:

1. create a new char[] to hold the result of the concatenation
2. create a new java.lang.String instance that is wrapped around the character array

If you do this repeatedly, especially when result is already long, this takes a lot of time.

If performance (both time and memory) is important for that method, you should consider using a StringBuilder (not thread-safe) or a StringBuffer (thread-safe).