1125947 - Fix -Wunreachable-code warning in netwerk/sctp/datachannel/DataChannel.cpp

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: Wunreachable_DataChannel.patch

Bug 807647 disabled the synchronous DataChannelConnection::SendPacket() code path by adding an `if (0)`, which causes this -Wunreachable-code warning:

netwerk/sctp/datachannel/DataChannel.cpp:717:11 [-Wunreachable-code] code will never be executed

SendPacket() is now only used asynchronously, so we can remove its always-true `bool release` parameter. We can also remove the `int res` error code because it is always 0.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8554672 [details] [diff] [review]
Wunreachable_DataChannel.patch

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

r-, since I don't want to lose all the code and commentary (and never fix this or forget to push them to deal with it).  If we need this, let's convert into an #ifdef, or leave the comment (mostly) intact, and reference a new bug where the code to handle synchronous calls has been moved to.

Comment 2

[Chris Peterson [:cpeterson]]

(In reply to Randell Jesup [:jesup] from comment #1)
> r-, since I don't want to lose all the code and commentary (and never fix
> this or forget to push them to deal with it).

Do we have a bug filed?


> If we need this, let's
> convert into an #ifdef, or leave the comment (mostly) intact, and reference
> a new bug where the code to handle synchronous calls has been moved to.

clang/gcc's -Wunreachable-code warning is not enabled by default, so fixing it is not critical.

Comment 3

[Chris Peterson [:cpeterson]]

Attachment: Wunreachable-code_DataChannel_v2.patch

I'd like to fix this DataChannel.cpp warning after all because I am trying to enable -Wunreachable-code warnings by default in mozilla-central.

Fix more clang warnings in netwerk/sctp/datachannel:

> netwerk/sctp/datachannel/DataChannel.cpp:727:11 [-Wunreachable-code] code will never be executed

Adding extra parens around the always-false conditional `0 /*peer->IsSTSThread()*/` suppresses the -Wunreachable-code warning about the SendPacket() never being reached.

> netwerk/sctp/datachannel/DataChannel.cpp:1835:19 [-Wshadow] declaration of 'i' shadows a previous local

This function has two for loops using two different index variables named `i`. Move the outer `i` inside the for loop scope to avoid the -Wshadow warning.

> netwerk/sctp/datachannel/DataChannel.h:531:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels

Suppress clang's -Wimplicit-fallthrough warnings (not yet enabled by default in mozilla-central) about switch cases that fall through without a break or return statement. The warnings can be suppressed with the MOZ_FALLTHROUGH, a MFBT macro for a clang compiler annotation. MOZ_FALLTHROUGH is only needed on cases that have code:

switch (foo) {
  case 1: // These cases have no code. No fallthrough annotations are needed.
  case 2:
  case 3:
    foo = 4; // This case has code, so a fallthrough annotation is needed:
    MOZ_FALLTHROUGH;
  default:
    return foo;
}

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee5539ede95d

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ee5539ede95d

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/ee5539ede95d

Comment 7

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion