[Freeciv-Dev] (PR#11771)

["Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=11771 >

I am having trouble running Freeciv on my laptop.  Some really weird 
errors occur, so I decided to do memcheck (valgrind).  I got a couple 
of errors and after a day of work traced them to the gen-specialists 
and the network.  I be Jason could have found the bug in 10 minutes, 
but oh well, at least I learnt how to use valgrind.  Annoyingly, it 
reported the error (accessing uninitialized values) only after they 
were copied over once already.

Now the bug: because of the constraints of the packets_gen, Jason was 
sending over the whole array specialist_bonus together wih unused 
(because typically num_specialist_types is less than SP_MAX) values.  
On one occasion it is harmless, on another a bit less so, because delta 
packets actually compare previous junk values with new junk values to 
determine if anything changed.

There are two ways to combat it:
1) introduce a new variable in the packet which is the upper limit of 
the array
2) clear the junk values before sending

There are two places I found and I fixed them in two different ways in 
the patch.  I leave it to Jason to select the better way although I 
would probably go for (1) since it is probably faster and results in 
less data being sent.

Sadly, I suspect this has very little relevance to the bad bugs I was 
seeing.

diff -ur -X freeciv/diff_ignore freeciv-orig/common/packets.def 
freeciv/common/packets.def
--- freeciv-orig/common/packets.def     2004-12-29 19:46:42.000000000 -0600
+++ freeciv/common/packets.def  2005-01-01 20:05:29.015388888 -0600
@@ -130,7 +130,7 @@
       possible. The array size in the "[]" can be specified plain as a
       term. In this case all elements will be transmitted. If this is
       not-desired you can specify the amount of elements to be
-      transfered by given the number. So the extenmded format is
+      transfered by given the number. So the extended format is
       "[<full-array-size>:<elements-to-transfer>]". elements-to-transfer
       is relative to the packet.
 
@@ -970,10 +970,11 @@
 
 PACKET_RULESET_GAME=97;sc,lsend
   UINT8 default_specialist, num_specialist_types;
+  UINT8 bonus_array_size;
   STRING specialist_name[SP_MAX:num_specialist_types][MAX_LEN_NAME];
   STRING specialist_short_name[SP_MAX:num_specialist_types][MAX_LEN_NAME];
   UINT8 specialist_min_size[SP_MAX:num_specialist_types];
-  UINT8 specialist_bonus[SP_MAX * O_MAX];
+  UINT8 specialist_bonus[SP_MAX * O_MAX:bonus_array_size];
   BOOL changable_tax;
   UINT8 forced_science;
   UINT8 forced_luxury;
diff -ur -X freeciv/diff_ignore freeciv-orig/server/citytools.c 
freeciv/server/citytools.c
--- freeciv-orig/server/citytools.c     2004-12-29 19:47:02.000000000 -0600
+++ freeciv/server/citytools.c  2005-01-01 20:05:17.198185376 -0600
@@ -1524,6 +1524,7 @@
     packet->ppl_unhappy[i]=pcity->ppl_unhappy[i];
     packet->ppl_angry[i]=pcity->ppl_angry[i];
   }
+  memset(packet->specialists, 0, sizeof(*(packet->specialists)) * SP_MAX);
   specialist_type_iterate(sp) {
     packet->specialists[sp] = pcity->specialists[sp];
   } specialist_type_iterate_end;
diff -ur -X freeciv/diff_ignore freeciv-orig/server/ruleset.c 
freeciv/server/ruleset.c
--- freeciv-orig/server/ruleset.c       2004-12-29 19:47:02.000000000 -0600
+++ freeciv/server/ruleset.c    2005-01-01 20:18:42.010835256 -0600
@@ -3062,6 +3062,7 @@
   struct packet_ruleset_game misc_p;
 
   misc_p.num_specialist_types = SP_COUNT;
+  misc_p.bonus_array_size = SP_COUNT * O_COUNT;
   misc_p.default_specialist = DEFAULT_SPECIALIST;
   specialist_type_iterate(sp) {
     int *bonus = game.rgame.specialists[sp].bonus;