Closed Bug 1158905 Opened 5 years ago Closed 5 years ago

remove dead code from protocol Transition functions

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Attachments

(1 file, 1 obsolete file)

PFoo::Transition currently looks something like:

bool
Transition(
        State from,
        mozilla::ipc::Trigger trigger,
        State* next)
{
    switch (from) {
    ...
    default:
        NS_RUNTIMEABORT("corrupted actor state");
        return false;
    }
    (*(next)) = __Error;
    return false;
}

Coverity complains that the assignment (*(next)) = __Error will never be
reached, and rightfully so.  Let's remove it.

Note that this is safe even with the "all --> Error transitions break to
here" comment; the compiler will complain if we do start caring about
state transitions and break out of the switch, because then we're going
to fall off the end of the function without returning a value.
This should also be a straightforward review, given the commit message.
Attachment #8598114 - Flags: review?(bent.mozilla)
Keywords: coverity
Attachment #8598114 - Flags: review?(bent.mozilla) → review+
boo, this doesn't fully work, because we have one (!) protocol that uses states:

http://mxr.mozilla.org/mozilla-central/source/dom/plugins/ipc/PPluginBackgroundDestroyer.ipdl#32

Fortunately, the compiler *does* warn about this! :)
Here's a version that only adds the fallthrough block if the protocol defines
any states.  Seems trivial, but probably worth a second look.

ipc/ipdl/ generated cpp files compile without warnings or errors now.
Attachment #8598114 - Attachment is obsolete: true
Attachment #8598240 - Flags: review?(bent.mozilla)
Comment on attachment 8598240 [details] [diff] [review]
remove dead code from protocol Transition functions

Review of attachment 8598240 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/ipdl/ipdl/lower.py
@@ +1837,5 @@
>                           init=ExprVar('mozilla::ipc::Trigger::Recv')))
>          if usesend or userecv:
>              transitionfunc.addstmt(Whitespace.NL)
>  
>          transitionfunc.addstmts([

Just use addstmt() here since you're only adding one now.
Attachment #8598240 - Flags: review?(bent.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/abdd54155023
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.