From a9031b6bae742b333b1b390192fa590f2ecb07ea Mon Sep 17 00:00:00 2001 From: peterbell10 Date: Mon, 5 Oct 2020 11:27:14 +0100 Subject: Fix cmake not adding Werror on clang, and _lots_ of warnings (#4963) * Fix cmake not adding Werror on clang, and _lots_ of warnings * WIP: Build fixes * Cannot make intermediate blockhandler instance * Tiger's changes * Fix BitIndex check * Handle invalid NextState values in cMultiVersionProtocol Co-authored-by: Tiger Wang --- src/Protocol/ChunkDataSerializer.cpp | 19 ++++++------ src/Protocol/ChunkDataSerializer.h | 4 +-- src/Protocol/Palettes/Palette_1_13.cpp | 2 +- src/Protocol/Palettes/Palette_1_13_1.cpp | 2 +- src/Protocol/Palettes/Palette_1_14.cpp | 2 +- src/Protocol/Palettes/Palette_1_15.cpp | 2 +- src/Protocol/Palettes/Palette_1_16.cpp | 4 +-- src/Protocol/Protocol.h | 1 - src/Protocol/ProtocolRecognizer.cpp | 52 +++++++++++++++++++++----------- src/Protocol/Protocol_1_12.cpp | 34 --------------------- src/Protocol/Protocol_1_13.cpp | 8 +++-- src/Protocol/Protocol_1_14.cpp | 32 +------------------- src/Protocol/Protocol_1_8.cpp | 42 +++++++++----------------- src/Protocol/Protocol_1_9.cpp | 17 ----------- 14 files changed, 71 insertions(+), 150 deletions(-) (limited to 'src/Protocol') diff --git a/src/Protocol/ChunkDataSerializer.cpp b/src/Protocol/ChunkDataSerializer.cpp index 2fd9e1cc2..9f8b91ac1 100644 --- a/src/Protocol/ChunkDataSerializer.cpp +++ b/src/Protocol/ChunkDataSerializer.cpp @@ -503,31 +503,30 @@ inline void cChunkDataSerializer::WriteSectionDataSeamless(const cChunkData::sCh ASSERT(a_BitsPerEntry < 64); UInt64 Buffer = 0; // A buffer to compose multiple smaller bitsizes into one 64-bit number - unsigned char BitIndex = 0; // The bit-position in Buffer that represents where to write next + int BitIndex = 0; // The bit-position in Buffer that represents where to write next for (size_t Index = 0; Index != cChunkData::SectionBlockCount; Index++) { - const UInt32 BlockType = a_Section.m_BlockTypes[Index]; - const UInt32 BlockMeta = (a_Section.m_BlockMetas[Index / 2] >> ((Index % 2) * 4)) & 0x0f; - const UInt32 Value = Palette(BlockType, BlockMeta); + const BLOCKTYPE BlockType = a_Section.m_BlockTypes[Index]; + const NIBBLETYPE BlockMeta = (a_Section.m_BlockMetas[Index / 2] >> ((Index % 2) * 4)) & 0x0f; + const auto Value = static_cast(Palette(BlockType, BlockMeta)); // Write as much as possible of Value, starting from BitIndex, into Buffer: - Buffer |= static_cast(Value) << BitIndex; + Buffer |= Value << BitIndex; // The _signed_ count of bits in Value left to write - const char Remaining = a_BitsPerEntry - (64 - BitIndex); - if (Remaining >= 0) + if (BitIndex + a_BitsPerEntry >= 64) { // There were some bits remaining: we've filled the buffer. Flush it: m_Packet.WriteBEUInt64(Buffer); // And write the remaining bits, setting the new BitIndex: - Buffer = Value >> (a_BitsPerEntry - Remaining); - BitIndex = Remaining; + Buffer = Value >> (64 - BitIndex); + BitIndex = a_BitsPerEntry - (64 - BitIndex); } else { - // It fit, sexcellent. + // It fit, excellent. BitIndex += a_BitsPerEntry; } } diff --git a/src/Protocol/ChunkDataSerializer.h b/src/Protocol/ChunkDataSerializer.h index cb39e27d6..aeff7c356 100644 --- a/src/Protocol/ChunkDataSerializer.h +++ b/src/Protocol/ChunkDataSerializer.h @@ -31,7 +31,7 @@ class cChunkDataSerializer v401, v477, - Count + Last = CacheVersion::v477 }; /** A single cache entry containing the raw data, compressed data, and a validity flag. */ @@ -79,7 +79,7 @@ protected: /** A cache, mapping protocol version to a fully serialised chunk. It is used during a single invocation of SendToClients with more than one client. */ - std::array(CacheVersion::Count)> m_Cache; + std::array(CacheVersion::Last) + 1> m_Cache; } ; diff --git a/src/Protocol/Palettes/Palette_1_13.cpp b/src/Protocol/Palettes/Palette_1_13.cpp index 7950ee72e..b6a8bbc0a 100644 --- a/src/Protocol/Palettes/Palette_1_13.cpp +++ b/src/Protocol/Palettes/Palette_1_13.cpp @@ -7916,7 +7916,7 @@ namespace Palette_1_13 case Statistic::WalkOneCm: return 5; case Statistic::WalkOnWaterOneCm: return 18; case Statistic::WalkUnderWaterOneCm: return 12; - default: return -1; + default: return UInt32(-1); } } diff --git a/src/Protocol/Palettes/Palette_1_13_1.cpp b/src/Protocol/Palettes/Palette_1_13_1.cpp index 25f02e47b..0786cf4fe 100644 --- a/src/Protocol/Palettes/Palette_1_13_1.cpp +++ b/src/Protocol/Palettes/Palette_1_13_1.cpp @@ -7932,7 +7932,7 @@ namespace Palette_1_13_1 case Statistic::WalkOneCm: return 5; case Statistic::WalkOnWaterOneCm: return 18; case Statistic::WalkUnderWaterOneCm: return 12; - default: return -1; + default: return UInt32(-1); } } diff --git a/src/Protocol/Palettes/Palette_1_14.cpp b/src/Protocol/Palettes/Palette_1_14.cpp index e98234ff2..29e9fcef9 100644 --- a/src/Protocol/Palettes/Palette_1_14.cpp +++ b/src/Protocol/Palettes/Palette_1_14.cpp @@ -9578,7 +9578,7 @@ namespace Palette_1_14 case Statistic::WalkOneCm: return 5; case Statistic::WalkOnWaterOneCm: return 8; case Statistic::WalkUnderWaterOneCm: return 12; - default: return -1; + default: return static_cast(-1); } } diff --git a/src/Protocol/Palettes/Palette_1_15.cpp b/src/Protocol/Palettes/Palette_1_15.cpp index cb07523a6..6cc67a488 100644 --- a/src/Protocol/Palettes/Palette_1_15.cpp +++ b/src/Protocol/Palettes/Palette_1_15.cpp @@ -9653,7 +9653,7 @@ namespace Palette_1_15 case Statistic::WalkOnWaterOneCm: return 8; case Statistic::WalkOneCm: return 5; case Statistic::WalkUnderWaterOneCm: return 12; - default: return -1; + default: return UInt32(-1); } } diff --git a/src/Protocol/Palettes/Palette_1_16.cpp b/src/Protocol/Palettes/Palette_1_16.cpp index 4d0c6fa39..baef7dfd0 100644 --- a/src/Protocol/Palettes/Palette_1_16.cpp +++ b/src/Protocol/Palettes/Palette_1_16.cpp @@ -12758,8 +12758,8 @@ namespace Palette_1_16 case Item::ZombieSpawnEgg: return 818; case Item::ZombieVillagerSpawnEgg: return 820; case Item::ZombiePigmanSpawnEgg: return 821; - default: return 0; } + UNREACHABLE("Invalid item"); } UInt32 From(const Statistic ID) @@ -12840,7 +12840,7 @@ namespace Palette_1_16 case Statistic::WalkOnWaterOneCm: return 8; case Statistic::WalkOneCm: return 5; case Statistic::WalkUnderWaterOneCm: return 12; - default: return -1; + default: return UInt32(-1); } } diff --git a/src/Protocol/Protocol.h b/src/Protocol/Protocol.h index cf3458040..5c9d5105f 100644 --- a/src/Protocol/Protocol.h +++ b/src/Protocol/Protocol.h @@ -350,7 +350,6 @@ public: Status = 1, Login = 2, Game = 3, - Invalid = 255 }; /** Called when client sends some data */ diff --git a/src/Protocol/ProtocolRecognizer.cpp b/src/Protocol/ProtocolRecognizer.cpp index b72f73f3d..94208cbf3 100644 --- a/src/Protocol/ProtocolRecognizer.cpp +++ b/src/Protocol/ProtocolRecognizer.cpp @@ -244,7 +244,7 @@ std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(c UInt32 ProtocolVersion; AString ServerAddress; UInt16 ServerPort; - cProtocol::State NextState; + UInt32 NextStateValue; if (!m_Buffer.ReadVarInt(PacketType) || (PacketType != 0x00)) { @@ -262,7 +262,7 @@ std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(c !m_Buffer.ReadVarInt(ProtocolVersion) || !m_Buffer.ReadVarUTF8String(ServerAddress) || !m_Buffer.ReadBEUInt16(ServerPort) || - !m_Buffer.ReadVarInt(NextState) + !m_Buffer.ReadVarInt(NextStateValue) ) { // TryRecognizeProtocol guarantees that we will have as much @@ -270,6 +270,22 @@ std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(c throw TriedToJoinWithUnsupportedProtocolException("Incorrect amount of data received - hacked client?"); } + cProtocol::State NextState = [&] + { + switch (NextStateValue) + { + case cProtocol::State::Status: return cProtocol::State::Status; + case cProtocol::State::Login: return cProtocol::State::Login; + case cProtocol::State::Game: return cProtocol::State::Game; + default: + { + throw TriedToJoinWithUnsupportedProtocolException( + fmt::format("Invalid next game state: {}", NextStateValue) + ); + } + } + }(); + // TODO: this should be a protocol property, not ClientHandle: a_Client.SetProtocolVersion(ProtocolVersion); @@ -285,23 +301,23 @@ std::unique_ptr cMultiVersionProtocol::TryRecognizeLengthedProtocol(c // All good, eat up the data: m_Buffer.CommitRead(); - switch (static_cast(ProtocolVersion)) + switch (ProtocolVersion) { - case cProtocol::Version::v1_8_0: return std::make_unique (&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_9_0: return std::make_unique (&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_9_1: return std::make_unique (&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_9_2: return std::make_unique (&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_9_4: return std::make_unique (&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_10_0: return std::make_unique(&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_11_0: return std::make_unique(&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_11_1: return std::make_unique(&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_12: return std::make_unique (&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_12_1: return std::make_unique(&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_12_2: return std::make_unique(&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_13: return std::make_unique (&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_13_1: return std::make_unique(&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_13_2: return std::make_unique(&a_Client, ServerAddress, NextState); - case cProtocol::Version::v1_14: return std::make_unique (&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_8_0): return std::make_unique (&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_9_0): return std::make_unique (&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_9_1): return std::make_unique (&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_9_2): return std::make_unique (&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_9_4): return std::make_unique (&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_10_0): return std::make_unique(&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_11_0): return std::make_unique(&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_11_1): return std::make_unique(&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_12): return std::make_unique (&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_12_1): return std::make_unique(&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_12_2): return std::make_unique(&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_13): return std::make_unique (&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_13_1): return std::make_unique(&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_13_2): return std::make_unique(&a_Client, ServerAddress, NextState); + case static_cast(cProtocol::Version::v1_14): return std::make_unique (&a_Client, ServerAddress, NextState); default: { LOGD("Client \"%s\" uses an unsupported protocol (lengthed, version %u (0x%x))", diff --git a/src/Protocol/Protocol_1_12.cpp b/src/Protocol/Protocol_1_12.cpp index 8a3076980..16651bdf6 100644 --- a/src/Protocol/Protocol_1_12.cpp +++ b/src/Protocol/Protocol_1_12.cpp @@ -1082,23 +1082,6 @@ bool cProtocol_1_12::HandlePacket(cByteBuffer & a_ByteBuffer, UInt32 a_PacketTyp } break; } - default: - { - // Received a packet in an unknown state, report: - LOGWARNING("Received a packet in an unknown protocol state %d. Ignoring further packets.", m_State); - - // Cannot kick the client - we don't know this state and thus the packet number for the kick packet - - // Switch to a state when all further packets are silently ignored: - m_State = State::Invalid; - return false; - } - case State::Invalid: - { - // This is the state used for "not processing packets anymore" when we receive a bad packet from a client. - // Do not output anything (the caller will do that for us), just return failure - return false; - } } // switch (m_State) // Unknown packet type, report to the ClientHandle: @@ -1229,23 +1212,6 @@ bool cProtocol_1_12_1::HandlePacket(cByteBuffer & a_ByteBuffer, UInt32 a_PacketT } break; } - default: - { - // Received a packet in an unknown state, report: - LOGWARNING("Received a packet in an unknown protocol state %d. Ignoring further packets.", m_State); - - // Cannot kick the client - we don't know this state and thus the packet number for the kick packet - - // Switch to a state when all further packets are silently ignored: - m_State = State::Invalid; - return false; - } - case State::Invalid: - { - // This is the state used for "not processing packets anymore" when we receive a bad packet from a client. - // Do not output anything (the caller will do that for us), just return failure - return false; - } } // switch (m_State) // Unknown packet type, report to the ClientHandle: diff --git a/src/Protocol/Protocol_1_13.cpp b/src/Protocol/Protocol_1_13.cpp index 0259565f5..1b505d58d 100644 --- a/src/Protocol/Protocol_1_13.cpp +++ b/src/Protocol/Protocol_1_13.cpp @@ -315,7 +315,9 @@ void cProtocol_1_13::HandlePacketSetBeaconEffect(cByteBuffer & a_ByteBuffer) { HANDLE_READ(a_ByteBuffer, ReadVarInt32, UInt32, Effect1); HANDLE_READ(a_ByteBuffer, ReadVarInt32, UInt32, Effect2); - m_Client->HandleBeaconSelection(Effect1, Effect2); + m_Client->HandleBeaconSelection( + static_cast(Effect1), static_cast(Effect2) + ); } @@ -655,7 +657,7 @@ bool cProtocol_1_13::ReadItem(cByteBuffer & a_ByteBuffer, cItem & a_Item, size_t return true; } - const auto Translated = GetItemFromProtocolID(ItemID); + const auto Translated = GetItemFromProtocolID(ToUnsigned(ItemID)); a_Item.m_ItemType = Translated.first; a_Item.m_ItemDamage = Translated.second; @@ -698,7 +700,7 @@ void cProtocol_1_13::WriteItem(cPacketizer & a_Pkt, const cItem & a_Item) } // Normal item - a_Pkt.WriteBEInt16(GetProtocolItemType(a_Item.m_ItemType, a_Item.m_ItemDamage)); + a_Pkt.WriteBEInt16(static_cast(GetProtocolItemType(a_Item.m_ItemType, a_Item.m_ItemDamage))); a_Pkt.WriteBEInt8(a_Item.m_ItemCount); // TODO: NBT diff --git a/src/Protocol/Protocol_1_14.cpp b/src/Protocol/Protocol_1_14.cpp index 335677155..a77cd2d7d 100644 --- a/src/Protocol/Protocol_1_14.cpp +++ b/src/Protocol/Protocol_1_14.cpp @@ -21,36 +21,6 @@ Implements the 1.14 protocol classes: -#define HANDLE_READ(ByteBuf, Proc, Type, Var) \ - Type Var; \ - do { \ - if (!ByteBuf.Proc(Var))\ - {\ - return;\ - } \ - } while (false) - - - - - -#define HANDLE_PACKET_READ(ByteBuf, Proc, Type, Var) \ - Type Var; \ - do { \ - { \ - if (!ByteBuf.Proc(Var)) \ - { \ - ByteBuf.CheckValid(); \ - return false; \ - } \ - ByteBuf.CheckValid(); \ - } \ - } while (false) - - - - - //////////////////////////////////////////////////////////////////////////////// // cProtocol_1_14: @@ -89,7 +59,7 @@ void cProtocol_1_14::SendLogin(const cPlayer & a_Player, const cWorld & a_World) Pkt.WriteBEInt32(static_cast(a_World.GetDimension())); Pkt.WriteBEUInt8(static_cast(Clamp(Server->GetMaxPlayers(), 0, 255))); Pkt.WriteString("default"); - Pkt.WriteVarInt32(a_World.GetMaxViewDistance()); + Pkt.WriteVarInt32(ToUnsigned(a_World.GetMaxViewDistance())); Pkt.WriteBool(false); } diff --git a/src/Protocol/Protocol_1_8.cpp b/src/Protocol/Protocol_1_8.cpp index ae3137eb6..5856a87d2 100644 --- a/src/Protocol/Protocol_1_8.cpp +++ b/src/Protocol/Protocol_1_8.cpp @@ -9,6 +9,7 @@ Implements the 1.8 protocol classes: #include "Globals.h" #include "Protocol_1_8.h" +#include "main.h" #include "../mbedTLS++/Sha1Checksum.h" #include "Packetizer.h" @@ -91,20 +92,13 @@ static const UInt32 CompressionThreshold = 256; // After how large a packet sho -// fwd: main.cpp: -extern bool g_ShouldLogCommIn, g_ShouldLogCommOut; - - - - - //////////////////////////////////////////////////////////////////////////////// // cProtocol_1_8_0: cProtocol_1_8_0::cProtocol_1_8_0(cClientHandle * a_Client, const AString & a_ServerAddress, State a_State) : Super(a_Client), - m_ServerAddress(a_ServerAddress), m_State(a_State), + m_ServerAddress(a_ServerAddress), m_IsEncrypted(false) { AStringVector Params; @@ -395,6 +389,13 @@ void cProtocol_1_8_0::SendDisconnect(const AString & a_Reason) Pkt.WriteString(Printf("{\"text\":\"%s\"}", EscapeString(a_Reason).c_str())); break; } + default: + { + FLOGERROR( + "Tried to send disconnect in invalid game state {0}", + static_cast(m_State) + ); + } } } @@ -1725,7 +1726,8 @@ bool cProtocol_1_8_0::CompressPacket(const AString & a_Packet, AString & a_Compr ---------------------------------------------- */ const UInt32 DataSize = 0; - const auto PacketSize = cByteBuffer::GetVarIntSize(DataSize) + UncompressedSize; + const auto PacketSize = static_cast( + cByteBuffer::GetVarIntSize(DataSize) + UncompressedSize); cByteBuffer LengthHeaderBuffer( cByteBuffer::GetVarIntSize(PacketSize) + @@ -1776,8 +1778,9 @@ bool cProtocol_1_8_0::CompressPacket(const AString & a_Packet, AString & a_Compr return false; } - const UInt32 DataSize = UncompressedSize; - const auto PacketSize = cByteBuffer::GetVarIntSize(DataSize) + CompressedSize; + const UInt32 DataSize = static_cast(UncompressedSize); + const auto PacketSize = static_cast( + cByteBuffer::GetVarIntSize(DataSize) + CompressedSize); cByteBuffer LengthHeaderBuffer( cByteBuffer::GetVarIntSize(PacketSize) + @@ -2272,23 +2275,6 @@ bool cProtocol_1_8_0::HandlePacket(cByteBuffer & a_ByteBuffer, UInt32 a_PacketTy } break; } - default: - { - // Received a packet in an unknown state, report: - LOGWARNING("Received a packet in an unknown protocol state %d. Ignoring further packets.", m_State); - - // Cannot kick the client - we don't know this state and thus the packet number for the kick packet - - // Switch to a state when all further packets are silently ignored: - m_State = State::Invalid; - return false; - } - case State::Invalid: - { - // This is the state used for "not processing packets anymore" when we receive a bad packet from a client. - // Do not output anything (the caller will do that for us), just return failure - return false; - } } // switch (m_State) // Unknown packet type, report to the ClientHandle: diff --git a/src/Protocol/Protocol_1_9.cpp b/src/Protocol/Protocol_1_9.cpp index c170227b5..a152a46cb 100644 --- a/src/Protocol/Protocol_1_9.cpp +++ b/src/Protocol/Protocol_1_9.cpp @@ -632,23 +632,6 @@ bool cProtocol_1_9_0::HandlePacket(cByteBuffer & a_ByteBuffer, UInt32 a_PacketTy } break; } - default: - { - // Received a packet in an unknown state, report: - LOGWARNING("Received a packet in an unknown protocol state %d. Ignoring further packets.", m_State); - - // Cannot kick the client - we don't know this state and thus the packet number for the kick packet - - // Switch to a state when all further packets are silently ignored: - m_State = State::Invalid; - return false; - } - case State::Invalid: - { - // This is the state used for "not processing packets anymore" when we receive a bad packet from a client. - // Do not output anything (the caller will do that for us), just return failure - return false; - } } // switch (m_State) // Unknown packet type, report to the ClientHandle: -- cgit v1.2.3