Complete.Org: Mailing Lists: Archives: freeciv-dev: September 2002:
[Freeciv-Dev] [PATCH] long bit strings (PR#2115)
Home

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

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] [PATCH] long bit strings (PR#2115)
From: jdorje@xxxxxxxxxxxxxxxxxxxxx
Date: Mon, 30 Sep 2002 13:32:55 -0700 (PDT)

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';

[Prev in Thread] Current Thread [Next in Thread]
  • [Freeciv-Dev] [PATCH] long bit strings (PR#2115), jdorje <=