<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39828 >
This patch has two parts. The quick and dirty solution is to treat
terrain just like resources: when the pointer is NULL, send -1. That
fixes the server crash.
- info.type = plrtile->terrain->index;
+ info.type = plrtile->terrain ? plrtile->terrain->index : -1;
===
But now you are sending bad data to the client! The second part is to
add validity checking to the client. This is a good idea in any case.
(Never trust network data.)
This reveals that a single tile is bad.
1: handle_tile_info() unknown terrain (1,25).
===
For some historic reason, the enum known_type is in terrain.h (where it's
never used). Move to tile.h (where is the value is returned). Adjusted
various #includes to match.
Also, add a few instances of tile_special_type_iterate() from S2_2.
===
Reminder, this only fixes the 2.1.0 shared vision, not the other bugs
reported in the same ticket.
Committed S2_1 revision 13916.
Committed S2_2 revision 13917.
Committed trunk revision 13918.
Final S2_2 changes for posterity (a bit different from S2_1, same as trunk):
Index: server/maphand.c
===================================================================
--- server/maphand.c (revision 13916)
+++ server/maphand.c (working copy)
@@ -614,17 +614,18 @@
conn_list_iterate(dest, pconn) {
struct player *pplayer = pconn->player;
- enum tile_special_type spe;
if (!pplayer && !pconn->observer) {
continue;
}
if (!pplayer || map_is_known_and_seen(ptile, pplayer, V_MAIN)) {
info.known = TILE_KNOWN;
- info.type = terrain_number(ptile->terrain);
- for (spe = 0; spe < S_LAST; spe++) {
+ info.type = ptile->terrain ? terrain_number(ptile->terrain) : -1;
+
+ tile_special_type_iterate(spe) {
info.special[spe] = BV_ISSET(ptile->special, spe);
- }
+ } tile_special_type_iterate_end;
+
info.resource = ptile->resource ? resource_number(ptile->resource) : -1;
info.continent = ptile->continent;
send_packet_tile_info(pconn, &info);
@@ -633,19 +634,23 @@
struct player_tile *plrtile = map_get_player_tile(ptile, pplayer);
info.known = TILE_KNOWN_FOGGED;
- info.type = terrain_number(plrtile->terrain);
- for (spe = 0; spe < S_LAST; spe++) {
+ info.type = plrtile->terrain ? terrain_number(plrtile->terrain) : -1;
+
+ tile_special_type_iterate(spe) {
info.special[spe] = BV_ISSET(plrtile->special, spe);
- }
+ } tile_special_type_iterate_end;
+
info.resource = plrtile->resource ? resource_number(plrtile->resource) :
-1;
info.continent = ptile->continent;
send_packet_tile_info(pconn, &info);
} else if (send_unknown) {
info.known = TILE_UNKNOWN;
info.type = -1;
- for (spe = 0; spe < S_LAST; spe++) {
+
+ tile_special_type_iterate(spe) {
info.special[spe] = FALSE;
- }
+ } tile_special_type_iterate_end;
+
info.resource = -1;
info.continent = 0;
send_packet_tile_info(pconn, &info);
Index: common/aicore/path_finding.h
===================================================================
--- common/aicore/path_finding.h (revision 13916)
+++ common/aicore/path_finding.h (working copy)
@@ -14,7 +14,7 @@
#define FC__PATH_FINDING_H
#include "map.h"
-#include "terrain.h"
+#include "tile.h"
#include "unit.h"
#include "unittype.h"
Index: common/tile.h
===================================================================
--- common/tile.h (revision 13916)
+++ common/tile.h (working copy)
@@ -20,6 +20,13 @@
#include "terrain.h"
#include "unitlist.h"
+/* network, order dependent */
+enum known_type {
+ TILE_UNKNOWN = 0,
+ TILE_KNOWN_FOGGED = 1,
+ TILE_KNOWN = 2,
+};
+
/* Convenience macro for accessing tile coordinates. This should only be
* used for debugging. */
#define TILE_XY(ptile) ((ptile) ? (ptile)->x : -1), \
Index: common/terrain.c
===================================================================
--- common/terrain.c (revision 13916)
+++ common/terrain.c (working copy)
@@ -496,14 +496,13 @@
****************************************************************************/
enum tile_special_type find_special_by_rule_name(const char *name)
{
- enum tile_special_type i;
-
assert(ARRAY_SIZE(tile_special_type_names) == S_LAST);
- for (i = 0; i < S_LAST; i++) {
+
+ tile_special_type_iterate(i) {
if (0 == strcmp(tile_special_type_names[i], name)) {
return i;
}
- }
+ } tile_special_type_iterate_end;
return S_LAST;
}
Index: common/terrain.h
===================================================================
--- common/terrain.h (revision 13916)
+++ common/terrain.h (working copy)
@@ -55,12 +55,10 @@
BV_DEFINE(bv_special, S_LAST_PLUS);
-/* currently only used in edithand.c */
#define tile_special_type_iterate(special) \
{ \
- enum tile_special_type special; \
- \
- for (special = 0; special < S_LAST; special++) {
+ enum tile_special_type special = 0; \
+ for (; special < S_LAST; special++) {
#define tile_special_type_iterate_end \
} \
@@ -114,10 +112,6 @@
#define TER_COUNT (TER_LAST)
#define TER_MAX 64 /* Changing this breaks network compatability. */
-enum known_type {
- TILE_UNKNOWN, TILE_KNOWN_FOGGED, TILE_KNOWN
-};
-
BV_DEFINE(bv_terrain_flags, TER_MAX);
enum mapgen_terrain_property {
Index: client/packhand.c
===================================================================
--- client/packhand.c (revision 13916)
+++ client/packhand.c (working copy)
@@ -2025,17 +2025,33 @@
**************************************************************************/
void handle_tile_info(struct packet_tile_info *packet)
{
+ bool tile_changed = FALSE;
+ bool known_changed = FALSE;
+ struct terrain *pterrain = terrain_by_number(packet->type);
struct tile *ptile = map_pos_to_tile(packet->x, packet->y);
enum known_type old_known = client_tile_get_known(ptile);
- bool tile_changed = FALSE;
- bool known_changed = FALSE;
- enum tile_special_type spe;
if (!ptile->terrain || terrain_number(ptile->terrain) != packet->type) {
tile_changed = TRUE;
- ptile->terrain = terrain_by_number(packet->type);
+ switch (old_known) {
+ case TILE_UNKNOWN:
+ ptile->terrain = pterrain;
+ break;
+ case TILE_KNOWN_FOGGED:
+ case TILE_KNOWN:
+ if (pterrain || TILE_UNKNOWN == packet->known) {
+ ptile->terrain = pterrain;
+ } else {
+ tile_changed = FALSE;
+ freelog(LOG_ERROR,
+ "handle_tile_info() unknown terrain (%d,%d).",
+ packet->x,
+ packet->y);
+ }
+ break;
+ };
}
- for (spe = 0; spe < S_LAST; spe++) {
+ tile_special_type_iterate(spe) {
if (packet->special[spe]) {
if (!tile_has_special(ptile, spe)) {
tile_set_special(ptile, spe);
@@ -2047,7 +2063,7 @@
tile_changed = TRUE;
}
}
- }
+ } tile_special_type_iterate_end;
if (NULL != ptile->resource) {
tile_changed = (resource_number(ptile->resource) != packet->resource);
@@ -2079,6 +2095,7 @@
vision_layer_iterate(v) {
BV_CLR(ptile->tile_seen[v], game.info.player_idx);
} vision_layer_iterate_end;
+
switch (packet->known) {
case TILE_KNOWN:
BV_SET(ptile->tile_known, game.info.player_idx);
@@ -2093,10 +2110,10 @@
break;
default:
freelog(LOG_ERROR,
- "handle_tile_info() unknown tile value %d.",
+ "handle_tile_info() invalid known (%d).",
packet->known);
break;
- }
+ };
}
if (packet->spec_sprite[0] != '\0') {
Index: client/climap.h
===================================================================
--- client/climap.h (revision 13916)
+++ client/climap.h (working copy)
@@ -15,7 +15,7 @@
#define FC__CLIMAP_H
#include "fc_types.h" /* enum direction8, struct tile */
-#include "terrain.h" /* enum known_type */
+#include "tile.h" /* enum known_type */
#define map_exists() (map.tiles != NULL)
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
