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

On 03/04/2008, Marko Lindqvist  wrote:
>   This should be fixable simply by adding ref_count for vision sites.

 Patch attached.


 - ML

diff -Nurd -X.diff_ignore freeciv/common/vision.c freeciv/common/vision.c
--- freeciv/common/vision.c	2008-03-08 16:32:49.000000000 +0200
+++ freeciv/common/vision.c	2008-04-03 03:01:51.000000000 +0300
@@ -17,6 +17,7 @@
 
 #include <assert.h>
 
+#include "log.h"
 #include "mem.h"
 #include "shared.h"
 
@@ -77,6 +78,11 @@
 ****************************************************************************/
 void free_vision_site(struct vision_site *psite)
 {
+  if (psite->ref_count > 0) {
+    /* Somebody still uses this vision site. Do not free */
+    return;
+  }
+  assert(psite->ref_count == 0);
   free(psite);
 }
 
@@ -119,3 +125,25 @@
   psite->size = pcity->size;
   sz_strlcpy(psite->name, city_name(pcity));
 }
+
+/****************************************************************************
+  Copy relevant information from one site struct to another.
+  Currently only user expects everything except ref_count to be copied.
+  If other kind of users are added, parameter defining copied information
+  set should be added.
+****************************************************************************/
+void copy_vision_site(struct vision_site *dest, struct vision_site *src)
+{
+  /* Copy everything except ref_count. */
+  strcpy(dest->name, src->name);
+  dest->location = src->location;
+  dest->owner = src->owner;
+  dest->identity = src->identity;
+  dest->size = src->size;
+  dest->border_radius_sq = src->border_radius_sq;
+  dest->occupied = src->occupied;
+  dest->walls = src->walls;
+  dest->happy = src->happy;
+  dest->unhappy = src->unhappy;
+  dest->improvements = src->improvements;
+}
diff -Nurd -X.diff_ignore freeciv/common/vision.h freeciv/common/vision.h
--- freeciv/common/vision.h	2008-03-08 16:32:49.000000000 +0200
+++ freeciv/common/vision.h	2008-04-03 00:05:41.000000000 +0300
@@ -124,6 +124,8 @@
   bool happy;
   bool unhappy;
 
+  int ref_count;
+
   bv_imprs improvements;
 };
 
@@ -134,6 +136,7 @@
 struct vision_site *create_vision_site_from_city(const struct city *pcity);
 void update_vision_site_from_city(struct vision_site *psite,
 				  const struct city *pcity);
+void copy_vision_site(struct vision_site *dest, struct vision_site *src);
 
 /* get 'struct site_list' and related functions: */
 #define SPECLIST_TAG site
diff -Nurd -X.diff_ignore freeciv/server/citytools.c freeciv/server/citytools.c
--- freeciv/server/citytools.c	2008-03-12 21:58:27.000000000 +0200
+++ freeciv/server/citytools.c	2008-04-03 03:32:24.000000000 +0300
@@ -1562,6 +1562,8 @@
       continue;
     }
     whole_map_iterate(ptile) {
+      /* FIXME: map_get_player_base()???
+       * Should be cities only: map_get_player_city() */
       if (!pplayer || NULL != map_get_player_base(ptile, pplayer)) {
 	send_city_info_at_tile(pplayer, pconn->self, NULL, ptile);
       }
@@ -1791,8 +1793,8 @@
   } improvement_iterate_end;
 
   if (NULL == pdcity) {
-    map_get_player_tile(pcenter, pplayer)->site =
     pdcity = create_vision_site_from_city(pcity);
+    change_playertile_site(map_get_player_tile(pcenter, pplayer), pdcity);
   } else if (pdcity->location != pcenter) {
     freelog(LOG_ERROR, "Trying to update bad city (wrong location)"
 	    " at %i,%i for player %s",
@@ -1840,8 +1842,11 @@
       struct player_tile *playtile = map_get_player_tile(ptile, pplayer);
 
       dlsend_packet_city_remove(pplayer->connections, pdcity->identity);
+      assert(playtile->site == pdcity);
       playtile->site = NULL;
-      free(pdcity);
+      pdcity->ref_count--; /* Subtract ref_count before calling
+                            * free_vision_site() */
+      free_vision_site(pdcity);
     }
   }
 }
diff -Nurd -X.diff_ignore freeciv/server/maphand.c freeciv/server/maphand.c
--- freeciv/server/maphand.c	2008-03-12 21:58:27.000000000 +0200
+++ freeciv/server/maphand.c	2008-04-03 03:36:35.000000000 +0300
@@ -1073,6 +1073,28 @@
 }
 
 /***************************************************************
+ Changes site information for player tile.
+***************************************************************/
+void change_playertile_site(struct player_tile *ptile,
+                            struct vision_site *new_site)
+{
+  if (ptile->site != NULL) {
+    /* Releasing old site from tile */
+    ptile->site->ref_count--;
+    assert(ptile->site->ref_count >= 0);
+    if (ptile->site->ref_count == 0) {
+      /* Free vision site before losing its address completely */
+      free_vision_site(ptile->site);
+    }
+  }
+  if (new_site != NULL) {
+    /* Assigning new site to tile */
+    new_site->ref_count++;
+  }
+  ptile->site = new_site;
+}
+
+/***************************************************************
 ...
 ***************************************************************/
 void map_set_known(struct tile *ptile, struct player *pplayer)
@@ -1148,8 +1170,8 @@
   whole_map_iterate(ptile) {
     struct player_tile *playtile = map_get_player_tile(ptile, pplayer);
 
-    /* cleverly uses return that is NULL for non-site tile */
-    playtile->site = map_get_player_base(ptile, pplayer);
+    /* map_get_player_base() will return NULL for non-site tile */
+    change_playertile_site(playtile, map_get_player_base(ptile, pplayer));
   } whole_map_iterate_end;
 
   /* only after removing borders! */
@@ -1157,6 +1179,8 @@
     struct vision_site *psite = map_get_player_base(ptile, pplayer);
 
     if (NULL != psite) {
+      /* Player tile will be freed, so ref_count goes down */
+      psite->ref_count--;
       free_vision_site(psite);
     }
   } whole_map_iterate_end;
@@ -1197,7 +1221,9 @@
 }
 
 /****************************************************************************
-  ...
+  Returns vision site located at given tile from player map.
+  FIXME: Rename function as it's not returning only bases, but
+         any vision sites.
 ****************************************************************************/
 struct vision_site *map_get_player_base(const struct tile *ptile,
 					const struct player *pplayer)
@@ -1211,7 +1237,7 @@
 }
 
 /****************************************************************************
-  ...
+  Returns city located at given tile from player map.
 ****************************************************************************/
 struct vision_site *map_get_player_city(const struct tile *ptile,
 					const struct player *pplayer)
@@ -1355,10 +1381,14 @@
       /* Set and send new city info */
       if (from_tile->site && from_tile->site->location == ptile) {
 	if (!dest_tile->site || dest_tile->site->location != ptile) {
-	  dest_tile->site = fc_calloc(1, sizeof(*dest_tile->site));
+	  change_playertile_site(dest_tile, NULL);
+          dest_tile->site = create_vision_site(0, NULL, NULL);
+          dest_tile->site->ref_count++;
 	}
-	/* struct assignment copy */
-	*dest_tile->site = *from_tile->site;
+	/* Copy vision information.
+         * Note that we don't care if receiver knows vision source city
+         * or not. */
+        copy_vision_site(dest_tile->site, from_tile->site);
 	send_city_info_at_tile(pdest, pdest->connections, NULL, ptile);
       }
 
@@ -1758,7 +1788,7 @@
     struct player_tile *playtile = map_get_player_tile(ptile, ploser);
 
     /* cleverly uses return that is NULL for non-site tile */
-    playtile->site = map_get_player_base(ptile, ploser);
+    change_playertile_site(playtile, map_get_player_base(ptile, ploser));
 
     if (NULL != playtile->site && ptile == psource) {
       /* has new owner */
@@ -1774,7 +1804,7 @@
       if (ptile != psource) {
         struct player_tile *playtile = map_get_player_tile(ptile, powner);
         assert(NULL == playtile->site);
-        playtile->site = psite;
+        change_playertile_site(playtile, psite);
       } else if (NULL != pcity) {
         update_vision_site_from_city(psite, pcity);
       } else {
@@ -1790,8 +1820,7 @@
       } else {
         psite = create_vision_site(IDENTITY_NUMBER_ZERO, psource, powner);
       }
-
-      playsite->site = psite;
+      change_playertile_site(playsite, psite);
     }
   } else {
     assert(NULL == powner && NULL == psource);
diff -Nurd -X.diff_ignore freeciv/server/maphand.h freeciv/server/maphand.h
--- freeciv/server/maphand.h	2008-03-10 19:54:21.000000000 +0200
+++ freeciv/server/maphand.h	2008-04-03 01:55:03.000000000 +0300
@@ -110,4 +110,7 @@
 			 int radius_sq);
 void vision_clear_sight(struct vision *vision);
 
+void change_playertile_site(struct player_tile *ptile,
+                            struct vision_site *new_site);
+
 #endif  /* FC__MAPHAND_H */
diff -Nurd -X.diff_ignore freeciv/server/savegame.c freeciv/server/savegame.c
--- freeciv/server/savegame.c	2008-03-25 10:46:50.000000000 +0200
+++ freeciv/server/savegame.c	2008-04-03 03:38:31.000000000 +0300
@@ -2835,7 +2835,7 @@
 
     for (i = 0; i < total_ncities; i++) {
       /* similar to create_vision_site() */
-      struct vision_site *pdcity = fc_calloc(1, sizeof(*pdcity));
+      struct vision_site *pdcity = create_vision_site(0, NULL, NULL);
 
       nat_y = secfile_lookup_int(file, "player%d.dc%d.y", plrno, i);
       nat_x = secfile_lookup_int(file, "player%d.dc%d.x", plrno, i);
@@ -2905,7 +2905,8 @@
       }
       sz_strlcpy(pdcity->name, secfile_lookup_str(file, "player%d.dc%d.name", plrno, i));
 
-      map_get_player_tile(pdcity->location, plr)->site = pdcity;
+      change_playertile_site(map_get_player_tile(pdcity->location, plr),
+                             pdcity);
       identity_number_reserve(pdcity->identity);
     }
 
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to