[freeciv-ai] Re: (PR#6236) Bug in CM

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Jason Short wrote:
> Per I. Mathisen wrote:
> 
>>On Sun, 21 Sep 2003, Jason Short wrote:
>>
>>
>>>>Valgrind gives this fairly often for the server:
>>>>==13287==  at 0x8067A58: auto_arrange_workers (cityturn.c:235)
>>
>>...
>>
>>
>>>>Could relate to PR#6222, but I doubt it (this is just an invalid read).
>>>>This may have been reported before. We really need to fix it.
>>>
>>>The culprit is the worker_positions_used field. In the client this is
>>>memset to 0 before it is used. In the server it's not.
>>
>>
>>I don't see why it should be. cm_query_result() shouldn't give us a
>>cm_result struct with unset data in it. So the bug must be in CM.
>>
>>I looked at this briefly but found nothing. Raimar?
> 
> my_city_map_iterate skips the center tile.
> 
> We should also remove the memset from the client cma code, I suspect.

OK, it looks like this is just a mistake in the order of checks by the 
calling code.

Although perhaps the CMA code should be changed as well, to be more robust.

Both this patch and the previous one prevent the valgrind warning. 
However it is now clear this warning is harmless.

jason

? rc
Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.223
diff -u -r1.223 cityturn.c
--- server/cityturn.c   2003/09/20 19:24:54     1.223
+++ server/cityturn.c   2003/09/22 15:43:46
@@ -233,13 +233,13 @@
   /* Now apply results */
   city_map_checked_iterate(pcity->x, pcity->y, x, y, mapx, mapy) {
     if (pcity->city_map[x][y] == C_TILE_WORKER
-        && !cmr.worker_positions_used[x][y]
-        && !is_city_center(x, y)) {
+       && !is_city_center(x, y)
+       && !cmr.worker_positions_used[x][y]) {
       server_remove_worker_city(pcity, x, y);
     }
     if (pcity->city_map[x][y] != C_TILE_WORKER
-        && cmr.worker_positions_used[x][y]
-        && !is_city_center(x, y)) {
+       && !is_city_center(x, y)
+       && cmr.worker_positions_used[x][y]) {
       server_set_worker_city(pcity, x, y);
     }
   } city_map_checked_iterate_end;