(svn r22178) [1.1] -Backport from trunk:

- Fix: [NewGRF] Memory leak if an industry NewGRF had more than one prop A or 15, or a station NewGRF had more than one prop 09 (r22175, r22165)
- Fix: [NewGRF] Disable a station NewGRF when it contains an unterminated spritelayout in action0 prop 08 instead of crashing (r22164)
- Fix: Building a station part adjacent to both an existing station and a rail waypoint failed [FS#4541] (r22163)

src/newgrf.cpp

@@ -1199,7 +1199,6 @@ static ChangeInfoResult StationChangeInfo(uint stid, int numinfo, int prop, Byte
 			case 0x09: // Define sprite layout
 				statspec->tiles = buf->ReadExtendedByte();
 				statspec->renderdata = CallocT<DrawTileSprites>(statspec->tiles);
-				statspec->copied_renderdata = false;
 
 				for (uint t = 0; t < statspec->tiles; t++) {
 					DrawTileSprites *dts = &statspec->renderdata[t];
@@ -1208,7 +1207,12 @@ static ChangeInfoResult StationChangeInfo(uint stid, int numinfo, int prop, Byte
 					dts->seq = NULL;
 					dts->ground.sprite = buf->ReadWord();
 					dts->ground.pal = buf->ReadWord();
-					if (dts->ground.sprite == 0) continue;
+					if (dts->ground.sprite == 0 && dts->ground.pal == 0) {
+						extern const DrawTileSprites _station_display_datas_rail[8];
+						dts->ground = _station_display_datas_rail[t % 8].ground;
+						dts->seq = CopyDrawTileSeqStruct(_station_display_datas_rail[t % 8].seq);
+						continue;
+					}
 					if (HasBit(dts->ground.pal, 15)) {
 						/* Use sprite from Action 1 */
 						ClrBit(dts->ground.pal, 15);
@@ -1217,7 +1221,7 @@ static ChangeInfoResult StationChangeInfo(uint stid, int numinfo, int prop, Byte
 
 					MapSpriteMappingRecolour(&dts->ground);
 
-					while (buf->HasData()) {
+					for (;;) {
 						/* no relative bounding box support */
 						dts->seq = ReallocT(const_cast<DrawTileSeqStruct *>(dts->seq), ++seq_count);
 						DrawTileSeqStruct *dtss = const_cast<DrawTileSeqStruct *>(&dts->seq[seq_count - 1]);
@@ -1254,8 +1258,11 @@ static ChangeInfoResult StationChangeInfo(uint stid, int numinfo, int prop, Byte
 				}
 
 				statspec->tiles = srcstatspec->tiles;
-				statspec->renderdata = srcstatspec->renderdata;
+				statspec->renderdata = MallocT<DrawTileSprites>(statspec->tiles);
-				statspec->copied_renderdata = true;
+				for (uint t = 0; t < statspec->tiles; t++) {
+					statspec->renderdata[t].ground = srcstatspec->renderdata[t].ground;
+					statspec->renderdata[t].seq = CopyDrawTileSeqStruct(srcstatspec->renderdata[t].seq);
+				}
 				break;
 			}
 
@@ -2524,6 +2531,20 @@ static bool ValidateIndustryLayout(const IndustryTileTable *layout, int size)
 	return true;
 }
 
+/** Clean the tile table of the IndustrySpec if it's needed. */
+static void CleanIndustryTileTable(IndustrySpec *ind)
+{
+	if (HasBit(ind->cleanup_flag, CLEAN_TILELAYOUT) && ind->table != NULL) {
+		for (int j = 0; j < ind->num_table; j++) {
+			/* remove the individual layouts */
+			free((void*)ind->table[j]);
+		}
+		/* remove the layouts pointers */
+		free((void*)ind->table);
+		ind->table = NULL;
+	}
+}
+
 static ChangeInfoResult IndustriesChangeInfo(uint indid, int numinfo, int prop, ByteReader *buf)
 {
 	ChangeInfoResult ret = CIR_SUCCESS;
@@ -2598,20 +2619,20 @@ static ChangeInfoResult IndustriesChangeInfo(uint indid, int numinfo, int prop,
 			}
 
 			case 0x0A: { // Set industry layout(s)
-				indsp->num_table = buf->ReadByte(); // Number of layaouts
+				byte new_num_layouts = buf->ReadByte(); // Number of layaouts
 				/* We read the total size in bytes, but we can't rely on the
 				 * newgrf to provide a sane value. First assume the value is
 				 * sane but later on we make sure we enlarge the array if the
 				 * newgrf contains more data. Each tile uses either 3 or 5
 				 * bytes, so to play it safe we assume 3. */
 				uint32 def_num_tiles = buf->ReadDWord() / 3 + 1;
-				IndustryTileTable **tile_table = CallocT<IndustryTileTable*>(indsp->num_table); // Table with tiles to compose an industry
+				IndustryTileTable **tile_table = CallocT<IndustryTileTable*>(new_num_layouts); // Table with tiles to compose an industry
 				IndustryTileTable *itt = CallocT<IndustryTileTable>(def_num_tiles); // Temporary array to read the tile layouts from the GRF
 				uint size;
 				const IndustryTileTable *copy_from;
 
 				try {
-					for (byte j = 0; j < indsp->num_table; j++) {
+					for (byte j = 0; j < new_num_layouts; j++) {
 						for (uint k = 0;; k++) {
 							if (k >= def_num_tiles) {
 								grfmsg(3, "IndustriesChangeInfo: Incorrect size for industry tile layout definition for industry %u.", indid);
@@ -2674,7 +2695,7 @@ static ChangeInfoResult IndustriesChangeInfo(uint indid, int numinfo, int prop,
 						if (!ValidateIndustryLayout(copy_from, size)) {
 							/* The industry layout was not valid, so skip this one. */
 							grfmsg(1, "IndustriesChangeInfo: Invalid industry layout for industry id %u. Ignoring", indid);
-							indsp->num_table--;
+							new_num_layouts--;
 							j--;
 						} else {
 							tile_table[j] = CallocT<IndustryTileTable>(size);
@@ -2682,7 +2703,7 @@ static ChangeInfoResult IndustriesChangeInfo(uint indid, int numinfo, int prop,
 						}
 					}
 				} catch (...) {
-					for (int i = 0; i < indsp->num_table; i++) {
+					for (int i = 0; i < new_num_layouts; i++) {
 						free(tile_table[i]);
 					}
 					free(tile_table);
@@ -2690,9 +2711,12 @@ static ChangeInfoResult IndustriesChangeInfo(uint indid, int numinfo, int prop,
 					throw;
 				}
 
+				/* Clean the tile table if it was already set by a previous prop A. */
+				CleanIndustryTileTable(indsp);
 				/* Install final layout construction in the industry spec */
+				indsp->num_table = new_num_layouts;
 				indsp->table = tile_table;
-				SetBit(indsp->cleanup_flag, 1);
+				SetBit(indsp->cleanup_flag, CLEAN_TILELAYOUT);
 				free(itt);
 				break;
 			}
@@ -2755,8 +2779,11 @@ static ChangeInfoResult IndustriesChangeInfo(uint indid, int numinfo, int prop,
 					throw;
 				}
 
+				if (HasBit(indsp->cleanup_flag, CLEAN_RANDOMSOUNDS)) {
+					free((void*)indsp->random_sounds);
+				}
 				indsp->random_sounds = sounds;
-				SetBit(indsp->cleanup_flag, 0);
+				SetBit(indsp->cleanup_flag, CLEAN_RANDOMSOUNDS);
 				break;
 			}
 
@@ -7031,13 +7058,10 @@ static void ResetCustomStations()
 			if (stations[i] == NULL) continue;
 			StationSpec *statspec = stations[i];
 
-			/* Release renderdata, if it wasn't copied from another custom station spec  */
+			for (uint t = 0; t < statspec->tiles; t++) {
-			if (!statspec->copied_renderdata) {
+				free((void*)statspec->renderdata[t].seq);
-				for (uint t = 0; t < statspec->tiles; t++) {
-					free((void*)statspec->renderdata[t].seq);
-				}
-				free(statspec->renderdata);
 			}
+			free(statspec->renderdata);
 
 			/* Release platforms and layouts */
 			if (!statspec->copied_layouts) {
@@ -7131,15 +7155,7 @@ static void ResetCustomIndustries()
 				}
 
 				/* We need to remove the tiles layouts */
-				if (HasBit(ind->cleanup_flag, CLEAN_TILELAYOUT) && ind->table != NULL) {
+				CleanIndustryTileTable(ind);
-					for (int j = 0; j < ind->num_table; j++) {
-						/* remove the individual layouts */
-						free((void*)ind->table[j]);
-					}
-					/* remove the layouts pointers */
-					free((void*)ind->table);
-					ind->table = NULL;
-				}
 
 				free(ind);
 			}

src/newgrf_station.h

@@ -78,7 +78,6 @@ struct StationSpec {
 	 */
 	uint tiles;
 	DrawTileSprites *renderdata; ///< Array of tile layouts.
-	bool copied_renderdata;
 
 	/**
 	 * Cargo threshold for choosing between little and lots of cargo

src/sprite.cpp

@@ -14,6 +14,8 @@
 #include "viewport_func.h"
 #include "landscape.h"
 #include "spritecache.h"
+#include "core/alloc_func.hpp"
+#include "core/mem_func.hpp"
 
 
 /**
@@ -108,3 +110,17 @@ void DrawCommonTileSeqInGUI(int x, int y, const DrawTileSprites *dts, int32 orig
 		}
 	}
 }
+
+/** Create a copy of an existing DrawTileSeqStruct array. */
+const DrawTileSeqStruct *CopyDrawTileSeqStruct(const DrawTileSeqStruct *dtss)
+{
+	const DrawTileSeqStruct *element;
+
+	size_t count = 1; // 1 for the terminator
+	foreach_draw_tile_seq(element, dtss) count++;
+
+	DrawTileSeqStruct *copy = MallocT<DrawTileSeqStruct>(count);
+	MemCpyT(copy, dtss, count);
+
+	return copy;
+}

src/sprite.h

@@ -34,6 +34,8 @@ struct DrawTileSeqStruct {
 	PalSpriteID image;
 };
 
+const DrawTileSeqStruct *CopyDrawTileSeqStruct(const DrawTileSeqStruct *dtss);
+
 /** Ground palette sprite of a tile, together with its child sprites */
 struct DrawTileSprites {
 	PalSpriteID ground;           ///< Palette and sprite for the ground

src/station_cmd.cpp

@@ -92,9 +92,10 @@ CommandCost GetStationAround(TileArea ta, StationID closest_station, T **st)
 	TILE_AREA_LOOP(tile_cur, ta) {
 		if (IsTileType(tile_cur, MP_STATION)) {
 			StationID t = GetStationIndex(tile_cur);
+			if (!T::IsValidID(t)) continue;
 
 			if (closest_station == INVALID_STATION) {
-				if (T::IsValidID(t)) closest_station = t;
+				closest_station = t;
 			} else if (closest_station != t) {
 				return_cmd_error(STR_ERROR_ADJOINS_MORE_THAN_ONE_EXISTING);
 			}

src/table/station_land.h

@@ -789,7 +789,7 @@ static const DrawTileSeqStruct _station_display_datas_waypoint_Y[] = {
 #define TILE_SPRITE_LINE(img, dtss) { {img, PAL_NONE}, dtss },
 #define TILE_SPRITE_NULL() { {0, 0}, NULL },
 
-static const DrawTileSprites _station_display_datas_rail[] = {
+extern const DrawTileSprites _station_display_datas_rail[] = {
 	TILE_SPRITE_LINE(SPR_RAIL_TRACK_X,               _station_display_datas_0)
 	TILE_SPRITE_LINE(SPR_RAIL_TRACK_Y,               _station_display_datas_1)
 	TILE_SPRITE_LINE(SPR_RAIL_TRACK_X,               _station_display_datas_2)