[GitHub] storm pull request #2655: fix STORM-3051

[lujiefsi <gi...@git.apache.org>]

GitHub user lujiefsi opened a pull request:

    https://github.com/apache/storm/pull/2655

    fix STORM-3051

    We have developed a static analysis tool NPEDetector to find some potential NPE. Our analysis shows that some callees may return null in corner case(e.g. node crash , IO exception), some of their callers have  !=null check but some do not have. 
    
    ### Bug:
    
    1.  callee CgroupCenter#getSubSystems return null when meet exception:
    <pre>
    } catch (Exception e) {
    LOG.error("Get subSystems error {}", e);
    }
    return null;
    </pre>
    but its caller use it without check:
    <pre>
    public boolean isSubSystemEnabled(SubSystemType subSystemType) {
      Set<SubSystem> subSystems = this.getSubSystems();
      for (SubSystem subSystem : subSystems) {
         if (subSystem.getType() == subSystemType) {
         return true;
      }
      }
      return false;
    }
    </pre>
    other callee and caller pair that have same problem.
    
    2. callee RAS_Node#getUsedSlots and caller RAS_Node#totalSlotsUsed
    3. CgroupCenter#getHierarchies and caller CgroupCenter#isMounted, 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lujiefsi/storm STORM-3051

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/2655.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2655
    
----
commit bfa55635110b11f9b04fa0861109c9b914b3508c
Author: LJ1043041006 <12...@...>
Date:   2018-05-02T11:01:41Z

    fix STORM-3051

----


---

[GitHub] storm pull request #2655: STORM-3051:some Potential NPEs

[HeartSaVioR <gi...@git.apache.org>]

Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2655#discussion_r186308155
  
    --- Diff: storm-client/src/jvm/org/apache/storm/container/cgroup/CgroupCenter.java ---
    @@ -97,9 +97,11 @@ public synchronized static CgroupCenter getInstance() {
         @Override
         public boolean isSubSystemEnabled(SubSystemType subSystemType) {
             Set<SubSystem> subSystems = this.getSubSystems();
    -        for (SubSystem subSystem : subSystems) {
    -            if (subSystem.getType() == subSystemType) {
    -                return true;
    +        if (subSystems != null){
    --- End diff --
    
    nit: space between `)` and `{`


---