widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #09137
[Merge] lp:~widelands-dev/widelands/bad_cast into lp:widelands
SirVer has proposed merging lp:~widelands-dev/widelands/bad_cast into lp:widelands.
Commit message:
Avoid accessing invalid memory by copying needed data to the stack.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1648178 in widelands: "Fatal Exception: Bad Cast"
https://bugs.launchpad.net/widelands/+bug/1648178
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bad_cast/+merge/313283
--
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bad_cast into lp:widelands.
=== modified file 'src/editor/map_generator.cc'
--- src/editor/map_generator.cc 2016-08-04 15:49:05 +0000
+++ src/editor/map_generator.cc 2016-12-14 22:13:01 +0000
@@ -95,16 +95,19 @@
// Set bob according to bob area
- if (set_immovable && (num = bobCategory->num_immovables()))
- egbase_.create_immovable(
+ if (set_immovable && (num = bobCategory->num_immovables())) {
+ egbase_.create_immovable_with_name(
fc, bobCategory->get_immovable(static_cast<size_t>(rng.rand() / (kMaxElevation / num))),
- MapObjectDescr::OwnerType::kWorld);
+ MapObjectDescr::OwnerType::kWorld, nullptr /* owner */, nullptr /* former_building_descr */
+ );
+ }
- if (set_moveable && (num = bobCategory->num_critters()))
+ if (set_moveable && (num = bobCategory->num_critters())) {
egbase_.create_critter(
fc, egbase_.world().get_bob(
bobCategory->get_critter(static_cast<size_t>(rng.rand() / (kMaxElevation / num)))
.c_str()));
+ }
}
void MapGenerator::generate_resources(uint32_t const* const random1,
=== modified file 'src/editor/tools/place_immovable_tool.cc'
--- src/editor/tools/place_immovable_tool.cc 2016-08-04 15:49:05 +0000
+++ src/editor/tools/place_immovable_tool.cc 2016-12-14 22:13:01 +0000
@@ -58,7 +58,8 @@
do {
if (!mr.location().field->get_immovable() &&
(mr.location().field->nodecaps() & Widelands::MOVECAPS_WALK))
- egbase.create_immovable(mr.location(), *i);
+ egbase.create_immovable(mr.location(), *i, Widelands::MapObjectDescr::OwnerType::kWorld,
+ nullptr /* owner */);
++i;
} while (mr.advance(*map));
}
@@ -84,7 +85,9 @@
immovable->remove(egbase);
}
if (!i->empty())
- egbase.create_immovable(mr.location(), *i);
+ egbase.create_immovable_with_name(
+ mr.location(), *i, Widelands::MapObjectDescr::OwnerType::kWorld, nullptr /* owner */,
+ nullptr /* former_building_descr */);
++i;
} while (mr.advance(*map));
=== modified file 'src/logic/editor_game_base.cc'
--- src/logic/editor_game_base.cc 2016-11-17 06:29:48 +0000
+++ src/logic/editor_game_base.cc 2016-12-14 22:13:01 +0000
@@ -326,37 +326,51 @@
Immovable& EditorGameBase::create_immovable(const Coords& c,
DescriptionIndex const idx,
MapObjectDescr::OwnerType type,
- const Building* former_building) {
- const ImmovableDescr& descr =
- *(type == MapObjectDescr::OwnerType::kTribe ? tribes().get_immovable_descr(idx) :
- world().get_immovable_descr(idx));
- assert(&descr);
- inform_players_about_immovable(Map::get_index(c, map().get_width()), &descr);
- return descr.create(*this, c, former_building);
+ Player* owner) {
+ return do_create_immovable(c, idx, type, owner, nullptr);
}
-Immovable& EditorGameBase::create_immovable(const Coords& c,
+Immovable& EditorGameBase::create_immovable_with_name(const Coords& c,
const std::string& name,
MapObjectDescr::OwnerType type,
- const Building* former_building) {
+ Player* owner,
+ const BuildingDescr* former_building_descr) {
DescriptionIndex idx;
if (type == MapObjectDescr::OwnerType::kTribe) {
idx = tribes().immovable_index(name.c_str());
if (!tribes().immovable_exists(idx)) {
throw wexception(
- "EditorGameBase::create_immovable(%i, %i): %s is not defined for the tribes", c.x, c.y,
- name.c_str());
+ "EditorGameBase::create_immovable_with_name(%i, %i): %s is not defined for the tribes",
+ c.x, c.y, name.c_str());
}
} else {
idx = world().get_immovable_index(name.c_str());
if (idx == INVALID_INDEX) {
throw wexception(
- "EditorGameBase::create_immovable(%i, %i): %s is not defined for the world", c.x, c.y,
- name.c_str());
+ "EditorGameBase::create_immovable_with_name(%i, %i): %s is not defined for the world",
+ c.x, c.y, name.c_str());
}
}
- return create_immovable(c, idx, type, former_building);
-}
+ return do_create_immovable(c, idx, type, owner, former_building_descr);
+}
+
+Immovable& EditorGameBase::do_create_immovable(const Coords& c,
+ DescriptionIndex const idx,
+ MapObjectDescr::OwnerType type,
+ Player* owner,
+ const BuildingDescr* former_building_descr) {
+ const ImmovableDescr& descr =
+ *(type == MapObjectDescr::OwnerType::kTribe ? tribes().get_immovable_descr(idx) :
+ world().get_immovable_descr(idx));
+ assert(&descr);
+ inform_players_about_immovable(Map::get_index(c, map().get_width()), &descr);
+ Immovable& immovable = descr.create(*this, c, former_building_descr);
+ if (owner != nullptr) {
+ immovable.set_owner(owner);
+ }
+ return immovable;
+}
+
/**
* Instantly create a ship at the given x/y location.
=== modified file 'src/logic/editor_game_base.h'
--- src/logic/editor_game_base.h 2016-11-17 06:08:27 +0000
+++ src/logic/editor_game_base.h 2016-12-14 22:13:01 +0000
@@ -144,12 +144,13 @@
Bob& create_critter(const Coords&, const std::string& name, Player* owner = nullptr);
Immovable& create_immovable(const Coords&,
DescriptionIndex idx,
- MapObjectDescr::OwnerType = MapObjectDescr::OwnerType::kWorld,
- const Building* former_building = nullptr);
- Immovable& create_immovable(const Coords&,
+ MapObjectDescr::OwnerType,
+ Player* owner);
+ Immovable& create_immovable_with_name(const Coords&,
const std::string& name,
- MapObjectDescr::OwnerType = MapObjectDescr::OwnerType::kWorld,
- const Building* former_building = nullptr);
+ MapObjectDescr::OwnerType,
+ Player* owner,
+ const BuildingDescr* former_building);
Bob& create_ship(const Coords&, int ship_type_idx, Player* owner = nullptr);
Bob& create_ship(const Coords&, const std::string& name, Player* owner = nullptr);
@@ -250,6 +251,12 @@
// sends notifications about this.
void change_field_owner(const FCoords& fc, PlayerNumber new_owner);
+ Immovable& do_create_immovable(const Coords& c,
+ DescriptionIndex const idx,
+ MapObjectDescr::OwnerType type,
+ Player* owner,
+ const BuildingDescr* former_building_descr);
+
uint32_t gametime_;
ObjectManager objects_;
=== modified file 'src/logic/map_objects/immovable.cc'
--- src/logic/map_objects/immovable.cc 2016-12-06 07:35:21 +0000
+++ src/logic/map_objects/immovable.cc 2016-12-14 22:13:01 +0000
@@ -329,8 +329,8 @@
*/
Immovable& ImmovableDescr::create(EditorGameBase& egbase,
const Coords& coords,
- const Building* former_building) const {
- Immovable& result = *new Immovable(*this, former_building);
+ const BuildingDescr* former_building_descr) const {
+ Immovable& result = *new Immovable(*this, former_building_descr);
result.position_ = coords;
result.init(egbase);
return result;
@@ -344,9 +344,9 @@
==============================
*/
-Immovable::Immovable(const ImmovableDescr& imm_descr, const Widelands::Building* former_building)
+Immovable::Immovable(const ImmovableDescr& imm_descr, const Widelands::BuildingDescr* former_building_descr)
: BaseImmovable(imm_descr),
- former_building_descr_(former_building ? &former_building->descr() : nullptr),
+ former_building_descr_(former_building_descr),
anim_(0),
animstart_(0),
program_(nullptr),
@@ -354,9 +354,6 @@
anim_construction_total_(0),
anim_construction_done_(0),
program_step_(0) {
- if (former_building != nullptr) {
- set_owner(former_building->get_owner());
- }
}
Immovable::~Immovable() {
@@ -869,9 +866,8 @@
if (bob) {
game.create_ship(c, type_name, player);
} else {
- Immovable& imm = game.create_immovable(c, type_name, owner_type);
- if (player)
- imm.set_owner(player);
+ Immovable& imm = game.create_immovable_with_name(
+ c, type_name, owner_type, player, nullptr /* former_building_descr */);
}
} else
immovable.program_step(game);
@@ -924,7 +920,8 @@
probability_to_grow(descr.terrain_affinity(), f, map, game.world().terrains())) {
MapObjectDescr::OwnerType owner_type = descr.owner_type();
immovable.remove(game); // Now immovable is a dangling reference!
- game.create_immovable(f, type_name, owner_type);
+ game.create_immovable_with_name(
+ f, type_name, owner_type, nullptr /* owner */, nullptr /* former_building_descr */);
} else {
immovable.program_step(game);
}
@@ -1024,7 +1021,8 @@
(new_location.field->nodecaps() & MOVECAPS_WALK) &&
logic_rand_as_double(&game) < probability_to_grow(descr.terrain_affinity(), new_location,
map, game.world().terrains())) {
- game.create_immovable(mr.location(), type_name, descr.owner_type());
+ game.create_immovable_with_name(mr.location(), type_name, descr.owner_type(),
+ nullptr /* owner */, nullptr /* former_building_descr */);
}
}
=== modified file 'src/logic/map_objects/immovable.h'
--- src/logic/map_objects/immovable.h 2016-11-22 19:11:01 +0000
+++ src/logic/map_objects/immovable.h 2016-12-14 22:13:01 +0000
@@ -140,7 +140,7 @@
ImmovableProgram const* get_program(const std::string&) const;
Immovable&
- create(EditorGameBase&, const Coords&, const Widelands::Building* former_building) const;
+ create(EditorGameBase&, const Coords&, const Widelands::BuildingDescr* former_building_descr) const;
MapObjectDescr::OwnerType owner_type() const {
return owner_type_;
@@ -202,9 +202,10 @@
MO_DESCR(ImmovableDescr)
public:
- /// If this immovable was created by a building, 'former_building' can be set in order to display
+ /// If this immovable was created by a building, 'former_building_desr' can be set in order to display
/// information about it.
- Immovable(const ImmovableDescr&, const Widelands::Building* former_building = nullptr);
+ Immovable(const ImmovableDescr&,
+ const Widelands::BuildingDescr* former_building_descr = nullptr);
~Immovable();
Coords get_position() const {
=== modified file 'src/logic/map_objects/tribes/building.cc'
--- src/logic/map_objects/tribes/building.cc 2016-12-03 13:32:28 +0000
+++ src/logic/map_objects/tribes/building.cc 2016-12-14 22:13:01 +0000
@@ -239,6 +239,7 @@
}
Building::~Building() {
+ log("#sirver Destroying building this: %p\n", this);
if (optionswindow_)
hide_options();
}
@@ -442,10 +443,14 @@
void Building::destroy(EditorGameBase& egbase) {
const bool fire = burn_on_destroy();
const Coords pos = position_;
+ Player* building_owner = get_owner();
+ const BuildingDescr* building_descr = &descr();
PlayerImmovable::destroy(egbase);
// We are deleted. Only use stack variables beyond this point
if (fire) {
- egbase.create_immovable(pos, "destroyed_building", MapObjectDescr::OwnerType::kTribe, this);
+ egbase.create_immovable_with_name(pos, "destroyed_building",
+ MapObjectDescr::OwnerType::kTribe, building_owner,
+ building_descr);
}
}
=== modified file 'src/logic/map_objects/tribes/worker.cc'
--- src/logic/map_objects/tribes/worker.cc 2016-11-03 07:20:57 +0000
+++ src/logic/map_objects/tribes/worker.cc 2016-12-14 22:13:01 +0000
@@ -774,8 +774,8 @@
Immovable& newimm = game.create_immovable(
pos, state.ivar2, state.svar1 == "tribe" ? MapObjectDescr::OwnerType::kTribe :
- MapObjectDescr::OwnerType::kWorld);
- newimm.set_owner(get_owner());
+ MapObjectDescr::OwnerType::kWorld,
+ get_owner());
if (action.iparam1 == Action::plantUnlessObject)
state.objvar1 = &newimm;
@@ -847,7 +847,7 @@
position,
t.get_resource_indicator(
rdescr, (rdescr && rdescr->detectable()) ? position.field->get_resources_amount() : 0),
- MapObjectDescr::OwnerType::kTribe);
+ MapObjectDescr::OwnerType::kTribe, nullptr /* owner */);
// Geologist also sends a message notifying the player
if (rdescr && rdescr->detectable() && position.field->get_resources_amount()) {
=== modified file 'src/map_io/s2map.cc'
--- src/map_io/s2map.cc 2016-12-03 13:32:28 +0000
+++ src/map_io/s2map.cc 2016-12-14 22:13:01 +0000
@@ -724,7 +724,8 @@
if (idx == Widelands::INVALID_INDEX) {
throw wexception("Missing immovable type %s", new_immovable_name.c_str());
}
- egbase.create_immovable(location, idx, Widelands::MapObjectDescr::OwnerType::kWorld);
+ egbase.create_immovable(
+ location, idx, Widelands::MapObjectDescr::OwnerType::kWorld, nullptr /* owner */);
};
uint8_t c;
=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc 2016-12-08 17:27:00 +0000
+++ src/scripting/lua_map.cc 2016-12-14 22:13:01 +0000
@@ -990,13 +990,15 @@
if (imm_idx == Widelands::INVALID_INDEX)
report_error(L, "Unknown world immovable <%s>", objname.c_str());
- m = &egbase.create_immovable(c->coords(), imm_idx, MapObjectDescr::OwnerType::kWorld);
+ m = &egbase.create_immovable(
+ c->coords(), imm_idx, MapObjectDescr::OwnerType::kWorld, nullptr /* owner */);
} else if (from_where == "tribes") {
DescriptionIndex const imm_idx = egbase.tribes().immovable_index(objname);
if (imm_idx == Widelands::INVALID_INDEX)
report_error(L, "Unknown tribes immovable <%s>", objname.c_str());
- m = &egbase.create_immovable(c->coords(), imm_idx, MapObjectDescr::OwnerType::kTribe);
+ m = &egbase.create_immovable(
+ c->coords(), imm_idx, MapObjectDescr::OwnerType::kTribe, nullptr /* owner */);
} else {
report_error(
L, "There are no immovables for <%s>. Use \"world\" or \"tribes\"", from_where.c_str());
Follow ups
