[jira] [Created] (REEF-768) Fix high-priority warnings reported by Findbugs

["Mariia Mykhailova (JIRA)" <ji...@apache.org>]

Mariia Mykhailova created REEF-768:
--------------------------------------

             Summary: Fix high-priority warnings reported by Findbugs
                 Key: REEF-768
                 URL: https://issues.apache.org/jira/browse/REEF-768
             Project: REEF
          Issue Type: Bug
            Reporter: Mariia Mykhailova
            Assignee: Mariia Mykhailova
            Priority: Minor


There is an apparent infinite recursive loop in {{org.apache.reef.wake.impl.OpaqueLocalIdentifier.toString()}}
{noformat}
  public String toString() {
    return "local-opaque-id://" + this.toString();
  }
{noformat}
I'm puzzled as to what was supposed to be here - maybe some kind of field which actually stored id?

Bad comparison of nonnegative value with -1 in {{org.apache.reef.wake.profiler.WakeProfiler.newSetVertex(Set) (line 74)}}
Should be replaced with 1, judging from the code

Field only ever set to null: {{org.apache.reef.wake.storage.SequentialFileReader.dest}}
Do we need to fix this, or maybe we should delete the class which we never use (and which would throw NullPointerException if ever used - so don't even need to go through deprecation)? If we choose to delete it, there is a bunch of related classes under {{org.apache.reef.wake.storage}} which are used only in this reader, so they might be deprecation material too

Boxing/unboxing to parse a primitive ...
We're using Integer.valueOf (which returns Integer), should use Integer.parseInt (which returns int).

USELESS_STRING In method {{org.apache.reef.javabridge.generic.JobDriver$ContextMessageHandler.onNext(ContextMessage)}}
{{message.get()}} is a {{byte[]}}; calling {{toString()}} on it produces something like [C@16f0472. Should use {{new String(message.get())}} instead.

{{org.apache.reef.javabridge.NativeInterop.HANDLERS}} is a mutable collection
Findbugs suggestion is to wrap the map in {{Collections.unmodifiableMap}} - no harm should be done

{{org.apache.reef.runtime.common.evaluator.context.ContextManager.removeContext(String)}} forces garbage collection; extremely dubious except in benchmarking code
The code has a `// TODO sure??` comment. We should either fix the comment to clarify our intention or remove the line

Possible null pointer dereference of {{HttpServerShellCmdtHandler.cmdOutput}} in {{org.apache.reef.examples.hellohttp.HttpServerShellCmdtHandler.onHttpRequest(ParsedHttpRequest, HttpServletResponse)}}
Indeed it looks like cmdOutput can be null after the loop; should we wrap writing to output stream in {{if (cmdOutput != null)}}?

Useless condition: it's known that {{this.failDriverOnEvaluatorLogErrors == true}} at this point
In method {{org.apache.reef.runtime.yarn.driver.restart.DFSEvaluatorPreserver.handleException(Exception, String, String)}}
Indeed, the method can be simplified



There is also a number of internationalization warnings 
DM_DEFAULT_ENCODING: Reliance on default encoding

Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly. 

Should set a specific encoding, probably UTF-8.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)