From 0b5579c7556fd717e0e0f68704b59a32d65b5b5d Mon Sep 17 00:00:00 2001 From: davidebeatrici Date: Sun, 21 Jan 2018 22:54:47 +0100 Subject: [PATCH] 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. --- src/Group.cpp | 18 +++++------- src/mumble/Cert.cpp | 1 + src/mumble/PulseAudio.cpp | 7 +++++ src/murmur/Server.cpp | 61 ++++++++++++++++++++++----------------- 4 files changed, 51 insertions(+), 36 deletions(-) diff --git a/src/Group.cpp b/src/Group.cpp index a62c36c66..2d59c3c27 100644 --- 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 index 8de74a2d3..803fbc7fa 100644 --- 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 index da289c017..4831c70d5 100644 --- 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 index 2042ab6ab..d9776d599 100644 --- a/src/murmur/Server.cpp +++ b/src/murmur/Server.cpp @@ -844,21 +844,25 @@ void Server::run() { MessageHandler::UDPMessageType msgType = static_cast((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((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;