FindBugs for BaseX - BaseX-Talk - mailman.uni-konstanz.de

26 Feb 2011


      2011/2/26 Leonard Wörteler leonard.woerteler@uni-konstanz.de:
...
Hi Godmar,
that bug was pretty easy to fix, and I have no idea how the superfluous
downcast passed all our tests. It was introduced in version 6.2.8.
Generally, downcasts are bad software engineering practice and can and
should be avoided or minimized by proper design. Either change the
method signature to let the compiler flag attends at passing
incompatible types, or add virtual methods to a superclass/interface,
or (last resort) use instanceof.
Findbugs is a tool that flags errors such as these; you may have heard of it.
Running Findbugs over the current codebase yields 563 warnings, some
of which are clear bugs.
I recommend you do that.
For example:
void delete(final int pre, final int size) {
    NSNode nd = root.find(pre);
    if(nd.pre == pre) nd = nd.par;
    if(nd == null) root = rootDummy;
    while(!nd.equals(rootDummy)) {
      nd.delete(pre, size);
      nd = nd.par;
    }
  }
if 'nd == null', 'nd.equals()' crashes. If it can't be null, then
'root = rootDummy' is bogus.
Or:
case GRANT:
        final CmdPerm perm = consume(CmdPerm.class, cmd);
        if(perm == null) help(null, cmd);
        final String db = key(ON, null) ? name(cmd) : null;
        key(TO, cmd);
        return db == null ? new Grant(perm, name(cmd)) :
          new Grant(perm, name(cmd), db);
if 'perm' is null, you crash in the 'Grant' constructor when calling
perm.toString(). If it can't be null, the check is bogus.
Or:
@Override
  public boolean uses(final Use u) {
    return u == Use.CTX && (def == FunDef.MB || def == FunDef.MB
        || def == FunDef.EVAL || def == FunDef.RUN) || super.uses(u);
  }
Did you mean to check for two different values instead of FunDef.MB twice.
Or:
// [CG] XQuery/Formatter: calendars and places are ignored
    if(cal != null || plc != null);
Did you mean to return if cal != null || plc != null rather than doing nothing?
Or in FTWrods.java
if(iat == null) {
          final FTLexer lex = new FTLexer(ftt.opt);
/* elided */
          if(iat == null) {
            iat = ia;
            tl = t;
          } else if(mode == FTMode.M_ALL || mode == FTMode.M_ALLWORDS) {
            iat = FTIndexIterator.intersect(ia, iat, 0);
            tl += t;
          } else {
            iat = FTIndexIterator.union(ia, iat);
            tl = Math.max(t, tl);
          }
          iat.tokenNum(++ctx.ftoknum);
        }
        return iat.more() ? new FTNode(iat.matches(), data, iat.next(),
            tl, iat.indexSize(), iat.score()) : null;
      }
The code in the else branch is dead - iat is always null where you
test it due to the dominating test on entry.  The 'intersect' and
'union' call is never made.
Etc. well worth the time fixing.
- Godmar