From: <geo...@us...> - 2009-11-06 06:00:41
|
Revision: 3258 http://freeorion.svn.sourceforge.net/freeorion/revision/?rev=3258&view=rev Author: geoffthemedio Date: 2009-11-06 06:00:33 +0000 (Fri, 06 Nov 2009) Log Message: ----------- -Added safety check / early exits to Order execution to avoid crashes -Made more system object getters safer Modified Paths: -------------- trunk/FreeOrion/universe/System.h trunk/FreeOrion/util/Order.cpp Modified: trunk/FreeOrion/universe/System.h =================================================================== --- trunk/FreeOrion/universe/System.h 2009-11-06 05:13:08 UTC (rev 3257) +++ trunk/FreeOrion/universe/System.h 2009-11-06 06:00:33 UTC (rev 3258) @@ -230,8 +230,9 @@ const Universe& universe = GetUniverse(); ObjectIDVec retval; for (ObjectMultimap::const_iterator it = m_objects.begin(); it != m_objects.end(); ++it) { - if (universe.Object(it->second)->Accept(UniverseObjectSubclassVisitor<typename boost::remove_const<T>::type>())) - retval.push_back(it->second); + if (const UniverseObject* obj = universe.Object(it->second)) + if (obj->Accept(UniverseObjectSubclassVisitor<typename boost::remove_const<T>::type>())) + retval.push_back(it->second); } return retval; } @@ -243,8 +244,9 @@ ObjectIDVec retval; std::pair<ObjectMultimap::const_iterator, ObjectMultimap::const_iterator> range = m_objects.equal_range(orbit); for (ObjectMultimap::const_iterator it = range.first; it != range.second; ++it) { - if (universe.Object(it->second)->Accept(UniverseObjectSubclassVisitor<typename boost::remove_const<T>::type>())) - retval.push_back(it->second); + if (const UniverseObject* obj = universe.Object(it->second)) + if (obj->Accept(UniverseObjectSubclassVisitor<typename boost::remove_const<T>::type>())) + retval.push_back(it->second); } return retval; } Modified: trunk/FreeOrion/util/Order.cpp =================================================================== --- trunk/FreeOrion/util/Order.cpp 2009-11-06 05:13:08 UTC (rev 3257) +++ trunk/FreeOrion/util/Order.cpp 2009-11-06 06:00:33 UTC (rev 3258) @@ -176,6 +176,10 @@ Fleet* fleet = 0; if (m_system_id != UniverseObject::INVALID_OBJECT_ID) { System* system = universe.Object<System>(m_system_id); + if (!system) { + Logger().errorStream() << "Empire attempted to create a new fleet in a nonexistant system"; + return; + } fleet = new Fleet(m_fleet_name, system->X(), system->Y(), EmpireID()); // an ID is provided to ensure consistancy between server and client universes universe.InsertID(fleet, m_new_id); @@ -187,7 +191,12 @@ } for (unsigned int i = 0; i < m_ship_ids.size(); ++i) { // verify that empire is not trying to take ships from somebody else's fleet - if (!universe.Object(m_ship_ids[i])->OwnedBy(EmpireID())) { + const Ship* ship = universe.Object<Ship>(m_ship_ids[i]); + if (!ship) { + Logger().errorStream() << "Empire attempted to create a new fleet with an invalid ship"; + return; + } + if (!ship->OwnedBy(EmpireID())) { Logger().errorStream() << "Empire attempted to create a new fleet with ships from another's fleet."; return; } @@ -344,9 +353,8 @@ Fleet* source_fleet = universe.Object<Fleet>(SourceFleet()); Fleet* target_fleet = universe.Object<Fleet>(DestinationFleet()); - // sanity check if (!source_fleet || !target_fleet) { - Logger().errorStream() << "Illegal fleet id specified in fleet merge order."; + Logger().errorStream() << "Empire attempted to move ships to or from a nonexistant fleet"; return; } @@ -447,6 +455,10 @@ } Planet* planet = universe.Object<Planet>(m_planet); + if (!planet) { + Logger().errorStream() << "Empire attempted to colonize a nonexistant planet"; + return; + } planet->SetPrimaryFocus(FOCUS_FARMING); planet->SetSecondaryFocus(FOCUS_FARMING); @@ -484,7 +496,16 @@ // look up the ship and fleet in question Ship* ship = universe.Object<Ship>(m_ship); + if (!ship) { + Logger().errorStream() << "Empire attempted to colonize with a nonexistant ship"; + return; + } + Fleet* fleet = universe.Object<Fleet>(ship->FleetID()); + if (!fleet) { + Logger().errorStream() << "Empire attempte to colonize with a ship that somehow doesn't have a fleet...?"; + return; + } // verify that empire issuing order owns specified fleet if (!fleet->OwnedBy(EmpireID())) { @@ -534,8 +555,17 @@ Universe& universe = GetUniverse(); Planet* planet = universe.Object<Planet>(m_planet); + if (!planet) { + Logger().errorStream() << "Attempting to undo a fleet colonize order with an invalid planet id"; + return false; + } Fleet* fleet = universe.Object<Fleet>(m_colony_fleet_id); + // not having a fleet is OK - it may have been removed if the colony ship was the last ship in the fleet Ship* ship = universe.Object<Ship>(m_ship); + if (!ship) { + Logger().errorStream() << "Attempting to under a fleet colonize order with an invalid ship id"; + return false; + } // if the fleet from which the colony ship came no longer exists or has moved, recreate it if (!fleet || fleet->SystemID() != ship->SystemID()) { |