(svn r6137) -Codechange: some very minor cleanups:

- Start using DeleteXXX for every pool item, not manually doing it
  - Use some wrapper to improve logic
  - Rewrote some pieces to improve logic

engine.c

@@ -503,10 +503,12 @@ static EngineRenew *GetEngineReplacement(EngineRenewList erl, EngineID engine)
 void RemoveAllEngineReplacement(EngineRenewList *erl)
 {
 	EngineRenew *er = (EngineRenew *)(*erl);
+	EngineRenew *next;
 
 	while (er) {
-		er->from = INVALID_ENGINE; // "Deallocate" elements
-		er = er->next;
+		next = er->next;
+		DeleteEngineRenew(er);
+		er = next;
 	}
 	*erl = NULL; // Empty list
 }
@@ -559,7 +561,7 @@ int32 RemoveEngineReplacement(EngineRenewList *erl, EngineID engine, uint32 flag
 					/* Cut this element out */
 					prev->next = er->next;
 				}
-				er->from = INVALID_ENGINE; // Deallocate
+				DeleteEngineRenew(er);
 			}
 			return 0;
 		}

engine.h

@@ -255,6 +255,11 @@ static inline bool IsValidEngineRenew(const EngineRenew *er)
 	return er->from != INVALID_ENGINE;
 }
 
+static inline void DeleteEngineRenew(EngineRenew *er)
+{
+	er->from = INVALID_ENGINE;
+}
+
 #define FOR_ALL_ENGINE_RENEWS_FROM(er, start) for (er = GetEngineRenew(start); er != NULL; er = (er->index + 1 < GetEngineRenewPoolSize()) ? GetEngineRenew(er->index + 1) : NULL) if (er->from != INVALID_ENGINE) if (IsValidEngineRenew(er))
 #define FOR_ALL_ENGINE_RENEWS(er) FOR_ALL_ENGINE_RENEWS_FROM(er, 0)
 

order.h

@@ -135,6 +135,12 @@ static inline bool IsValidOrder(const Order *o)
 	return o->type != OT_NOTHING;
 }
 
+static inline void DeleteOrder(Order *o)
+{
+	o->type = OT_NOTHING;
+	o->next = NULL;
+}
+
 #define FOR_ALL_ORDERS_FROM(order, start) for (order = GetOrder(start); order != NULL; order = (order->index + 1 < GetOrderPoolSize()) ? GetOrder(order->index + 1) : NULL) if (IsValidOrder(order))
 #define FOR_ALL_ORDERS(order) FOR_ALL_ORDERS_FROM(order, 0)
 

order_cmd.c

@@ -1028,7 +1028,7 @@ bool VehicleHasDepotOrders(const Vehicle *v)
  */
 void DeleteVehicleOrders(Vehicle *v)
 {
-	Order *order, *cur;
+	Order *cur, *next;
 
 	DeleteOrderWarnings(v);
 
@@ -1066,20 +1066,10 @@ void DeleteVehicleOrders(Vehicle *v)
 	v->orders = NULL;
 	v->num_orders = 0;
 
-	order = NULL;
-	while (cur != NULL) {
-		if (order != NULL) {
-			order->type = OT_NOTHING;
-			order->next = NULL;
-		}
-
-		order = cur;
-		cur = cur->next;
-	}
-
-	if (order != NULL) {
-		order->type = OT_NOTHING;
-		order->next = NULL;
+		while (cur != NULL) {
+		next = cur->next;
+		DeleteOrder(cur);
+		cur = next;
 	}
 }
 

signs.c

@@ -179,11 +179,9 @@ int32 CmdRenameSign(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 		if (flags & DC_EXEC) {
 			Sign *si = GetSign(p1);
 
-			/* Delete the name */
-			DeleteName(si->str);
-			si->str = 0;
-
 			MarkSignDirty(si);
+			DeleteSign(si);
+
 			InvalidateWindow(WC_SIGN_LIST, 0);
 			_sign_sort_dirty = true;
 		}

signs.h

@@ -57,6 +57,12 @@ static inline bool IsValidSignID(uint index)
 	return index < GetSignPoolSize() && IsValidSign(GetSign(index));
 }
 
+static inline void DeleteSign(Sign *si)
+{
+	DeleteName(si->str);
+	si->str = STR_NULL;
+}
+
 #define FOR_ALL_SIGNS_FROM(ss, start) for (ss = GetSign(start); ss != NULL; ss = (ss->index + 1 < GetSignPoolSize()) ? GetSign(ss->index + 1) : NULL) if (IsValidSign(ss))
 #define FOR_ALL_SIGNS(ss) FOR_ALL_SIGNS_FROM(ss, 0)
 

town_cmd.c

@@ -1062,13 +1062,12 @@ bool GenerateTowns(void)
 
 	// give it a last try, but now more aggressive
 	if (num == 0 && CreateRandomTown(10000, 0) == NULL) {
-		if (GetTownArraySize() > 0) return true;
+		if (GetTownArraySize() == 0) {
+			/* XXX - can we handle that more gracefully? */
+			if (_game_mode != GM_EDITOR) error("Could not generate any town");
 
-		//XXX can we handle that more gracefully?
-		if (num == 0 && _game_mode != GM_EDITOR) {
-			error("Could not generate any town");
+			return false;
 		}
-		return false;
 	}
 
 	return true;

train.h

@@ -173,19 +173,6 @@ static inline void ClearMultiheaded(Vehicle *v)
 	CLRBIT(v->subtype, Train_Multiheaded);
 }
 
-/** Get the next real (non-articulated part) vehicle in the consist.
- * @param v Vehicle.
- * @return Next vehicle in the consist.
- */
-static inline Vehicle *GetNextVehicle(const Vehicle *v)
-{
-	Vehicle *u = v->next;
-	while (u != NULL && IsArticulatedPart(u)) {
-		u = u->next;
-	}
-	return u;
-}
-
 /** Check if an engine has an articulated part.
  * @param v Vehicle.
  * @return True if the engine has an articulated part.
@@ -195,16 +182,39 @@ static inline bool EngineHasArticPart(const Vehicle *v)
 	return (v->next != NULL && IsArticulatedPart(v->next));
 }
 
+/**
+ * Get the next part of a multi-part engine.
+ * Will only work on a multi-part engine (EngineHasArticPart(v) == true),
+ * Result is undefined for normal engine.
+ */
+static inline Vehicle *GetNextArticPart(const Vehicle *v)
+{
+	assert(EngineHasArticPart(v));
+	return v->next;
+}
+
 /** Get the last part of a multi-part engine.
  * @param v Vehicle.
  * @return Last part of the engine.
  */
 static inline Vehicle *GetLastEnginePart(Vehicle *v)
 {
-	while (EngineHasArticPart(v)) v = v->next;
+	while (EngineHasArticPart(v)) v = GetNextArticPart(v);
 	return v;
 }
 
+/** Get the next real (non-articulated part) vehicle in the consist.
+ * @param v Vehicle.
+ * @return Next vehicle in the consist.
+ */
+static inline Vehicle *GetNextVehicle(const Vehicle *v)
+{
+	while (EngineHasArticPart(v)) v = GetNextArticPart(v);
+
+	/* v now contains the last artic part in the engine */
+	return v->next;
+}
+
 void ConvertOldMultiheadToNew(void);
 void ConnectMultiheadedTrains(void);
 

vehicle.h

@@ -430,7 +430,9 @@ static inline Vehicle *GetFirstVehicleFromSharedList(Vehicle *v)
 VARDEF VehicleID _new_vehicle_id;
 VARDEF uint16 _returned_refit_capacity;
 
-#define INVALID_VEHICLE 0xFFFF
+enum {
+	INVALID_VEHICLE = 0xFFFF,
+};
 
 /**
  * Get the colour map for an engine. This used for unbuilt engines in the user interface.