You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "James E. King, III (JIRA)" <ji...@apache.org> on 2017/05/08 12:56:04 UTC

[jira] [Created] (THRIFT-4191) Java code generator equals() has sub-optimal generated paths (Coverity)

James E. King, III created THRIFT-4191:
------------------------------------------

             Summary: Java code generator equals() has sub-optimal generated paths (Coverity)
                 Key: THRIFT-4191
                 URL: https://issues.apache.org/jira/browse/THRIFT-4191
             Project: Thrift
          Issue Type: Bug
          Components: Java - Compiler
    Affects Versions: 0.10.0
            Reporter: James E. King, III
            Priority: Trivial


If you make a thrift file with a struct containing a single i32 and then generate java code for it, the equals() generated method will contain the following - this is annotated by Coverity to show the issue:

{noformat}

    	assignment: Assigning: this_present_interRequestPriority = true.
 813    boolean this_present_interRequestPriority = true;
    	assignment: Assigning: that_present_interRequestPriority = true.
 814    boolean that_present_interRequestPriority = true;
    	CID 26017 (#1 of 2): &#39;Constant&#39; variable guards dead code (DEADCODE) [select issue]
    	cond_const: Condition this_present_interRequestPriority, taking true branch. Now the value of this_present_interRequestPriority is equal to 1.
 815    if (this_present_interRequestPriority || that_present_interRequestPriority) {
    	const: At condition this_present_interRequestPriority, the value of this_present_interRequestPriority must be equal to 1.
    	dead_error_condition: The condition this_present_interRequestPriority must be true.
    	const: At condition that_present_interRequestPriority, the value of that_present_interRequestPriority must be equal to 1.
    	dead_error_condition: The condition that_present_interRequestPriority must be true.
 816      if (!(this_present_interRequestPriority && that_present_interRequestPriority))
    	
CID 26017 (#2 of 2): 'Constant' variable guards dead code (DEADCODE)
dead_error_line: Execution cannot reach this statement: return false;.
    	Local variable that_present_interRequestPriority is assigned only once, to a constant value, making it effectively constant throughout its scope. If this is not the intent, examine the logic to see if there is a missing assignment that would make that_present_interRequestPriority not remain constant. Otherwise, declaring that_present_interRequestPriority as final will suppress this defect.
 817        return false;
 818      if (this.interRequestPriority != that.interRequestPriority)
 819        return false;
 820    }
{noformat}

Since "interRequestPriority" is not an optional i32, I assume we're skipping the call to that.isSetInterRequestPriority().  If both are always true, we can remove the unnecessary branch or make it final as Coverity suggested.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)