[crossfire] SockList buffer size difference in server/client. Bug?

[Mark Wedel]

On 09/30/10 08:05 PM, Kevin R. Bulgrien wrote:
> server/trunk/include/shared/newclient.h:
>
> /**
>   * Maximum size of a packet the client expects to get.  Using a buffer of this
>   * size allows the client to avoid constant allocation and deallocation of the
>   * same buffer over and over again (at the cost of using extra memory).  This
>   * also makes the code simpler.  The size is big enough to receive any valid
>   * packet: 2 bytes for length, 65535 for maximum packet size, 1 for appended a
>   * trailing null character.
>   */
> #define MAXSOCKBUF (2+65535+1)
>
>    .  .  .
>
> /**
>   * Contains the base information we use to make up a packet we want to send.
>   */
> typedef struct SockList {
> #ifdef CLIENT_TYPES_H                       /* Used by the client */
>      int len;
>      unsigned char *buf;
> #else                                       /* Used by the server */
>      size_t len;
>      unsigned char buf[2+65536UL+1]; /* 2=length, 65536=content, 1=trailing
> NUL */
> #endif
> } SockList;
>
> Is use of 65536 in server vs 65535 in the client a bug?
>
> In client, the buffer is malloc'd in main().  It almost seems that it could
> just as well be statically allocated there too... doesn't seem like there
> is much point in the ifdef here.

  Realistically, I believe 65535 + 2 would always be sufficient.  If the length 
of a packet is greater than that, it can not be held in the 2 byte length parameter.

  I'm guessing the extra byte just allows one to always put a null one character 
beyond the end of the buffer - I'm not actually sure if that is really done or not.

  I'm not sure why the server has it be 65536 - that said, realistically, as 
long as it is 65538 (65535 + 2 + 1), it probably doesn't make any difference - 
if the server defined it as 100000, it would still work - just extra space is 
now being allocated which will never be used.

  That extra byte in this case almost certainly doesn't make a difference, as I 
suspect in most cases, the data following it is going to get aligned on a 4 byte 
boundary, which would bring that to 65540