From f2f4dc7de862cfa706c8388e3c3cd76742a8a9a6 Mon Sep 17 00:00:00 2001 From: Ben Longbons Date: Thu, 15 Sep 2011 05:15:48 +0800 Subject: Improve handling of packet lengths. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Use a symbol, VAR, instead of -1 for variable-length packets. * Also change it's value to 1, so the length can be properly unsigned. Note: A packet can't actually have length 1, since packet ID is 2 bytes. * Use correct type (uint16_t) for packet id and length in more places. * Avoid reading beyond the length of the array. * Immediately parse variable length packets with length 4 (i.e. no body) instead of waiting for another byte to arrive first. Reviewed-by: Thorbjørn Lindeijer --- src/net/tmwa/network.cpp | 88 +++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 42 deletions(-) (limited to 'src/net') diff --git a/src/net/tmwa/network.cpp b/src/net/tmwa/network.cpp index 44559170..2660eb50 100644 --- a/src/net/tmwa/network.cpp +++ b/src/net/tmwa/network.cpp @@ -37,7 +37,10 @@ /** Warning: buffers and other variables are shared, so there can be only one connection active at a time */ -short packet_lengths[] = { +// indicator for a variable-length packet +const uint16_t VAR = 1; + +uint16_t packet_lengths[0x220] = { 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -45,38 +48,38 @@ short packet_lengths[] = { // #0x0040 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 50, 3, -1, 55, 17, 3, 37, 46, -1, 23, -1, 3,108, 3, 2, - 3, 28, 19, 11, 3, -1, 9, 5, 54, 53, 58, 60, 41, 2, 6, 6, + 0, 50, 3,VAR, 55, 17, 3, 37, 46,VAR, 23,VAR, 3,108, 3, 2, + 3, 28, 19, 11, 3,VAR, 9, 5, 54, 53, 58, 60, 41, 2, 6, 6, // #0x0080 - 7, 3, 2, 2, 2, 5, 16, 12, 10, 7, 29, 23, -1, -1, -1, 0, - 7, 22, 28, 2, 6, 30, -1, -1, 3, -1, -1, 5, 9, 17, 17, 6, - 23, 6, 6, -1, -1, -1, -1, 8, 7, 6, 7, 4, 7, 0, -1, 6, - 8, 8, 3, 3, -1, 6, 6, -1, 7, 6, 2, 5, 6, 44, 5, 3, + 7, 3, 2, 2, 2, 5, 16, 12, 10, 7, 29, 23,VAR,VAR,VAR, 0, + 7, 22, 28, 2, 6, 30,VAR,VAR, 3,VAR,VAR, 5, 9, 17, 17, 6, + 23, 6, 6,VAR,VAR,VAR,VAR, 8, 7, 6, 7, 4, 7, 0,VAR, 6, + 8, 8, 3, 3,VAR, 6, 6,VAR, 7, 6, 2, 5, 6, 44, 5, 3, // #0x00C0 - 7, 2, 6, 8, 6, 7, -1, -1, -1, -1, 3, 3, 6, 6, 2, 27, - 3, 4, 4, 2, -1, -1, 3, -1, 6, 14, 3, -1, 28, 29, -1, -1, + 7, 2, 6, 8, 6, 7,VAR,VAR,VAR,VAR, 3, 3, 6, 6, 2, 27, + 3, 4, 4, 2,VAR,VAR, 3,VAR, 6, 14, 3,VAR, 28, 29,VAR,VAR, 30, 30, 26, 2, 6, 26, 3, 3, 8, 19, 5, 2, 3, 2, 2, 2, - 3, 2, 6, 8, 21, 8, 8, 2, 2, 26, 3, -1, 6, 27, 30, 10, + 3, 2, 6, 8, 21, 8, 8, 2, 2, 26, 3,VAR, 6, 27, 30, 10, // #0x0100 - 2, 6, 6, 30, 79, 31, 10, 10, -1, -1, 4, 6, 6, 2, 11, -1, + 2, 6, 6, 30, 79, 31, 10, 10,VAR,VAR, 4, 6, 6, 2, 11,VAR, 10, 39, 4, 10, 31, 35, 10, 18, 2, 13, 15, 20, 68, 2, 3, 16, - 6, 14, -1, -1, 21, 8, 8, 8, 8, 8, 2, 2, 3, 4, 2, -1, - 6, 86, 6, -1, -1, 7, -1, 6, 3, 16, 4, 4, 4, 6, 24, 26, + 6, 14,VAR,VAR, 21, 8, 8, 8, 8, 8, 2, 2, 3, 4, 2,VAR, + 6, 86, 6,VAR,VAR, 7,VAR, 6, 3, 16, 4, 4, 4, 6, 24, 26, // #0x0140 - 22, 14, 6, 10, 23, 19, 6, 39, 8, 9, 6, 27, -1, 2, 6, 6, - 110, 6, -1, -1, -1, -1, -1, 6, -1, 54, 66, 54, 90, 42, 6, 42, - -1, -1, -1, -1, -1, 30, -1, 3, 14, 3, 30, 10, 43, 14,186,182, - 14, 30, 10, 3, -1, 6,106, -1, 4, 5, 4, -1, 6, 7, -1, -1, + 22, 14, 6, 10, 23, 19, 6, 39, 8, 9, 6, 27,VAR, 2, 6, 6, + 110, 6,VAR,VAR,VAR,VAR,VAR, 6,VAR, 54, 66, 54, 90, 42, 6, 42, + VAR,VAR,VAR,VAR,VAR, 30,VAR, 3, 14, 3, 30, 10, 43, 14,186,182, + 14, 30, 10, 3,VAR, 6,106,VAR, 4, 5, 4,VAR, 6, 7,VAR,VAR, // #0x0180 - 6, 3,106, 10, 10, 34, 0, 6, 8, 4, 4, 4, 29, -1, 10, 6, + 6, 3,106, 10, 10, 34, 0, 6, 8, 4, 4, 4, 29,VAR, 10, 6, 90, 86, 24, 6, 30, 102, 9, 4, 8, 4, 14, 10, 4, 6, 2, 6, - 3, 3, 35, 5, 11, 26, -1, 4, 4, 6, 10, 12, 6, -1, 4, 4, - 11, 7, -1, 67, 12, 18,114, 6, 3, 6, 26, 26, 26, 26, 2, 3, + 3, 3, 35, 5, 11, 26,VAR, 4, 4, 6, 10, 12, 6,VAR, 4, 4, + 11, 7,VAR, 67, 12, 18,114, 6, 3, 6, 26, 26, 26, 26, 2, 3, // #0x01C0 - 2, 14, 10, -1, 22, 22, 4, 2, 13, 97, 0, 9, 9, 29, 6, 28, - 8, 14, 10, 35, 6, 8, 4, 11, 54, 53, 60, 2, -1, 47, 33, 6, - 30, 8, 34, 14, 2, 6, 26, 2, 28, 81, 6, 10, 26, 2, -1, -1, - -1, -1, 20, 10, 32, 9, 34, 14, 2, 6, 48, 56, -1, 4, 5, 10, + 2, 14, 10,VAR, 22, 22, 4, 2, 13, 97, 0, 9, 9, 29, 6, 28, + 8, 14, 10, 35, 6, 8, 4, 11, 54, 53, 60, 2,VAR, 47, 33, 6, + 30, 8, 34, 14, 2, 6, 26, 2, 28, 81, 6, 10, 26, 2,VAR,VAR, + VAR,VAR, 20, 10, 32, 9, 34, 14, 2, 6, 48, 56,VAR, 4, 5, 10, // #0x0200 26, 0, 0, 0, 18, 0, 0, 0, 0, 0, 0, 19, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -276,23 +279,24 @@ void Network::skip(int len) bool Network::messageReady() { - int len = -1, msgId; - MutexLocker lock(&mMutex); - if (mInSize >= 2) - { - msgId = readWord(0); - if (msgId == SMSG_SERVER_VERSION_RESPONSE) - len = 10; - else - len = packet_lengths[msgId]; - - if (len == -1 && mInSize > 4) - len = readWord(2); + if (mInSize < 2) + return false; + uint16_t msgId = readWord(0); + uint16_t len = 0; + // TODO don't hard-code this single case + if (msgId == SMSG_SERVER_VERSION_RESPONSE) + len = 10; + else if (msgId < 0x220) + len = packet_lengths[msgId]; + if (len == VAR) + { + if (mInSize < 4) + return false; + len = readWord(2); } - - return mInSize >= static_cast(len); + return mInSize >= len; } MessageIn Network::getNextMessage() @@ -304,14 +308,14 @@ MessageIn Network::getNextMessage() } MutexLocker lock(&mMutex); - int msgId = readWord(0); - int len; + uint16_t msgId = readWord(0); + uint16_t len = 0; if (msgId == SMSG_SERVER_VERSION_RESPONSE) len = 10; - else + else if (msgId < 0x220) len = packet_lengths[msgId]; - if (len == -1) + if (len == VAR) len = readWord(2); #ifdef DEBUG -- cgit v1.2.3-70-g09d2