CL_ParseServerMessage possible bug? (netquake)

[metlslime]

I just saw this last night... can't figure out how it's not a bug.

So in CL_ParseServerMessage we see that a message type of -1 is used to indicate the end of the packet:

Code: Select all

if (cmd == -1)
{
        SHOWNET("END OF MESSAGE");
        return;         // end of message
}

And we see that if the highest bit is set, that means it's an entity update:

Code: Select all

if (cmd & 128)
{
        SHOWNET("fast update");
        CL_ParseUpdate (cmd&127);
        continue;
}

And when we get to CL_ParseUpdate, we see that the other 7 bits are used to indicate various fields differ from the entity's baseline (defined in protocol.h):

Code: Select all

#define U_MOREBITS      (1<<0)
#define U_ORIGIN1       (1<<1)
#define U_ORIGIN2       (1<<2)
#define U_ORIGIN3       (1<<3)
#define U_ANGLE2        (1<<4)
#define U_NOLERP        (1<<5)
#define U_FRAME         (1<<6)

So the question is, if all 7 of these flags are set for legitimate reasons, the resulting bit pattern will be 11111111, which is -1, which is also used to indicate the end of the packet. So this is a bug, right? The parsing will end prematurely and the rest of the packet will be ignored, resulting in missing entities, sounds, etc...

[mh]

It's read as unsigned and promoted to an int so it's OK.

Code: Select all

   int         cmd;

...
...
...

      cmd = MSG_ReadByte ();

...
...
...

int MSG_ReadByte (void)
{
   int     c;
   
   if (msg_readcount+1 > net_message.cursize)
   {
      msg_badread = true;
      return -1;
   }
      
   c = (unsigned char)net_message.data[msg_readcount];
   msg_readcount++;
   
   return c;
}

[metlslime]

It's read as unsigned and promoted to an int so it's OK.

Code: Select all

if (msg_readcount+1 > net_message.cursize)
   {
      msg_badread = true;
      return -1;
   }

AHHH.... of course.