[Freeciv-Dev] [PATCH] long bit strings (PR#2115)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

This patch allows long bit strings (up to 65535 bits, instead of just 
254 bits) to be sent by dio_[put|get]_bit_string.  It also adds some 
slight error handling for the (inconceivable) case where a too-long bit 
string is sent (and the program is compiled with NDEBUG).
The patch breaks network compatability, so a mandatory capability of 
long_bitstrings is added.  Breaking compatability is not strictly 
manditory, but it is much simpler (see below), and not a big deal since 
it was recently broken anyway.
We could avoid breaking compatability using optional capabilities. 
However, this would take some rework of the dio code, since (AFAICT - 
and I have not looked too closely) the DIO code does not have access to 
the connection's capability string.  Also, additional error handling 
would be necessary since if you sent a bit string to an older client 
things would still break.
I used uint16 (unsigned short) instead of uint32 (unsigned int) since 
with the latter there would be additional complications (since the 
length of the bit string is currently stored in an int; size_t might be 
unsigned int; and the math could break).
A more generalized system to give unlimited bit string length could 
append multiple bit strings if the given bit string is too long.  At the 
receiving end, a flag would be necessary to label this.  But this seems 
unnecessarily complex; 65535 bits should be enough for anybody :-).
The use of (unsigned short)(-1) is a bit of a hack, but there does not 
seem to be a USHORT_MAX value to use in its stead.  There is a 
UINT16_MAX, but it doesn't look portable (?).
Finally, note that the comparison before was (bits < UCHAR_MAX), whereas 
now it is effectively (bits <= USHORT_MAX).  This change is fine, since 
the problem occurs when the length won't fit into a uint8/uint16.

Increasing the maximum bit string length is not strictly necessary, but 
will make FreeCiv slightly more stable in the long run.  And now is a 
good time to make it, since another manditory capability was recently added.
jason

Index: common/capstr.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/capstr.c,v
retrieving revision 1.110
diff -u -r1.110 capstr.c
--- common/capstr.c     2002/09/28 21:58:16     1.110
+++ common/capstr.c     2002/09/30 20:24:36
@@ -71,7 +71,7 @@
  */
 
 #define CAPABILITY "+1.13.0 conn_info turn_founded unitbv +freeze_thaw "\
-                   "+civ2happy"
+                   "+civ2happy +long_bitstrings"
   
 /* "+1.13.0" is protocol for 1.13.0 release.
   
@@ -86,6 +86,10 @@
     "freeze_thaw" uses PACKET_FREEZE_HINT/PACKET_THAW_HINT.
 
     "civ2happy" changes the way specialists are taken from workers
+
+    "long_bitstrings" means bit strings sent over the network can be
+    up to 65k bits; see dio_put_bit_string/dio_get_bit_string in
+    dataio.c.
 */
 
 void init_our_capability(void)
Index: common/dataio.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/dataio.c,v
retrieving revision 1.2
diff -u -r1.2 dataio.c
--- common/dataio.c     2002/09/30 15:27:46     1.2
+++ common/dataio.c     2002/09/30 20:24:36
@@ -339,14 +339,20 @@
 {
   /* Note that size_t is often an unsigned type, so we must be careful
    * with the math when calculating 'bytes'. */
-  size_t bits = strlen(value), bytes = (bits + 7) / 8;
+  size_t bits = strlen(value), bytes;
+  size_t max = (unsigned short)(-1);
 
-  assert(bits < UCHAR_MAX);
+  if (bits > max) {
+    freelog(LOG_ERROR, "Bit string too long: %d bits.", bits);
+    assert(FALSE);
+    bits = max;
+  }
+  bytes = (bits + 7) / 8;
 
   if (enough_space(dout, bytes + 1)) {
     size_t i;
 
-    dio_put_uint8(dout, bits);
+    dio_put_uint16(dout, bits);
 
     for (i = 0; i < bits;) {
       int bit, data = 0;
@@ -587,7 +593,7 @@
     return;
   }
 
-  dio_get_uint8(din, &npack);
+  dio_get_uint16(din, &npack);
   if (npack >= max_dest_size) {
     din->bad_bit_string = TRUE;
     dest[0] = '\0';