Improve handling of packet lengths.

* 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 <thorbjorn@lindeijer.nl>

1 files changed, 46 insertions, 42 deletions

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<unsigned int>(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