Fix and refactor problems found via GCC 7's -Wimplicit-fallthrough.

This commit fixes various problems discovered by building on Debian buster which uses GCC 7. Building on buster gave various -Wimplicit-fallthrough warnings. In Group.cpp and Server.cpp, we now use if statements instead of a switch with fallthrough. This improves readability of the code, and fixes the implicit fallthroughs. In PulseAudio.cpp, we had unintended fallthroughs in a switch statement, which have been fixed. In Cert.cpp, we had a potential fallthrough which was impossible to trigger at runtime, fixed by adding an error return, like the surrounding code. Fixes #3306.
mumble-voip · Jan 21, 2018 · 0b5579c · 0b5579c
1 parent 2c24ee0
commit 0b5579c
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 36 deletions.
diff --git a/src/Group.cpp b/src/Group.cpp
@@ -164,16 +164,14 @@ bool Group::isMember(Channel *curChan, Channel *aclChan, QString name, ServerUse
 		int maxdesc = 1000;
 		int minpath = 0;
 		QStringList args = name.split(QLatin1String(","));
-		switch (args.count()) {
-			default:
-			case 3:
-				maxdesc = args[2].isEmpty() ? maxdesc : args[2].toInt();
-			case 2:
-				mindesc = args[1].isEmpty() ? mindesc : args[1].toInt();
-			case 1:
-				minpath = args[0].isEmpty() ? minpath : args[0].toInt();
-			case 0:
-				break;
+		if (args.count() >= 3) {
+			maxdesc = args[2].isEmpty() ? maxdesc : args[2].toInt();
+		}
+		if (args.count() >= 2) {
+			mindesc = args[1].isEmpty() ? mindesc : args[1].toInt();
+		}
+		if (args.count() >= 1) {
+			minpath = args[0].isEmpty() ? minpath : args[0].toInt();
 		}
 
 		Channel *home = pl->cChannel;

diff --git a/src/mumble/Cert.cpp b/src/mumble/Cert.cpp
@@ -135,6 +135,7 @@ int CertWizard::nextId() const {
 					return 2;
 				else if (qrbExport->isChecked())
 					return 3;
+				return -1;
 			}
 		case 2: // Import
 			if (validateCert(kpCurrent))

diff --git a/src/mumble/PulseAudio.cpp b/src/mumble/PulseAudio.cpp
@@ -201,6 +201,8 @@ void PulseAudioSystem::eventCallback(pa_mainloop_api *api, pa_defer_event *) {
 						pasOutput = pa_stream_new(pacContext, mumble_sink_input, &pss, (pss.channels == 1) ? NULL : &pcm);
 						pa_stream_set_state_callback(pasOutput, stream_callback, this);
 						pa_stream_set_write_callback(pasOutput, write_callback, this);
+
+						break;
 					}
 				case PA_STREAM_UNCONNECTED:
 					do_start = true;
@@ -268,7 +270,10 @@ void PulseAudioSystem::eventCallback(pa_mainloop_api *api, pa_defer_event *) {
 						pasInput = pa_stream_new(pacContext, "Microphone", &pss, NULL);
 						pa_stream_set_state_callback(pasInput, stream_callback, this);
 						pa_stream_set_read_callback(pasInput, read_callback, this);
+
+						break;
 					}
+
 				case PA_STREAM_UNCONNECTED:
 					do_start = true;
 					break;
@@ -330,6 +335,8 @@ void PulseAudioSystem::eventCallback(pa_mainloop_api *api, pa_defer_event *) {
 						pasSpeaker = pa_stream_new(pacContext, mumble_echo, &pss, (pss.channels == 1) ? NULL : &pcm);
 						pa_stream_set_state_callback(pasSpeaker, stream_callback, this);
 						pa_stream_set_read_callback(pasSpeaker, read_callback, this);
+
+						break;
 					}
 				case PA_STREAM_UNCONNECTED:
 					do_start = true;

diff --git a/src/murmur/Server.cpp b/src/murmur/Server.cpp
@@ -844,21 +844,25 @@ void Server::run() {
 
 				MessageHandler::UDPMessageType msgType = static_cast<MessageHandler::UDPMessageType>((buffer[0] >> 5) & 0x7);
 
-				switch (msgType) {
-					case MessageHandler::UDPVoiceSpeex:
-					case MessageHandler::UDPVoiceCELTAlpha:
-					case MessageHandler::UDPVoiceCELTBeta:
-						if (bOpus)
-							break;
-					case MessageHandler::UDPVoiceOpus: {
-							u->aiUdpFlag = 1;
-							processMsg(u, buffer, len);
-							break;
-						}
-					case MessageHandler::UDPPing: {
-							QByteArray qba;
-							sendMessage(u, buffer, len, qba, true);
-						}
+				if (msgType == MessageHandler::UDPVoiceSpeex ||
+				    msgType == MessageHandler::UDPVoiceCELTAlpha ||
+				    msgType == MessageHandler::UDPVoiceCELTBeta ||
+				    msgType == MessageHandler::UDPVoiceOpus) {
+
+					// Allow all voice packets through by default.
+					bool ok = true;
+					// ...Unless we're in Opus mode. In Opus mode, only Opus packets are allowed.
+					if (bOpus && msgType != MessageHandler::UDPVoiceOpus) {
+						ok = false;
+					}
+
+					if (ok) {
+						u->aiUdpFlag = 1;
+						processMsg(u, buffer, len);
+					}
+				} else if (msgType == MessageHandler::UDPPing) {
+					QByteArray qba;
+					sendMessage(u, buffer, len, qba, true);
 				}
 #ifdef Q_OS_UNIX
 				fds[i].revents = 0;
@@ -1464,17 +1468,22 @@ void Server::message(unsigned int uiType, const QByteArray &qbaMsg, ServerUser *
 
 		MessageHandler::UDPMessageType msgType = static_cast<MessageHandler::UDPMessageType>((buffer[0] >> 5) & 0x7);
 
-		switch (msgType) {
-			case MessageHandler::UDPVoiceCELTAlpha:
-			case MessageHandler::UDPVoiceCELTBeta:
-			case MessageHandler::UDPVoiceSpeex:
-				if (bOpus)
-					break;
-			case MessageHandler::UDPVoiceOpus:
-				processMsg(u, buffer, l);
-				break;
-			default:
-				break;
+		if (msgType == MessageHandler::UDPVoiceSpeex ||
+		    msgType == MessageHandler::UDPVoiceCELTAlpha ||
+		    msgType == MessageHandler::UDPVoiceCELTBeta ||
+		    msgType == MessageHandler::UDPVoiceOpus) {
+
+			// Allow all voice packets through by default.
+			bool ok = true;
+			// ...Unless we're in Opus mode. In Opus mode, only Opus packets are allowed.
+			if (bOpus && msgType != MessageHandler::UDPVoiceOpus) {
+				ok = false;
+			}
+
+			if (ok) {
+				u->aiUdpFlag = 1;
+				processMsg(u, buffer, 1);
+			}
 		}
 
 		return;