Archives/OpenTTD-patches
2
0
Fork 0
mirror of https://github.com/JGRennison/OpenTTD-patches.git synced 2024-11-11 13:10:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Departures: Fix manual memory management, avoid leaks

Browse Source
This commit is contained in:
Jonathan G Rennison 2024-08-13 01:36:59 +01:00
parent 3717b5b934
commit 4c3072e719
4 changed files with 58 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
src/departures.cpp
View File

@ -214,12 +214,12 @@ static bool VehicleSetNextDepartureTime(Ticks *previous_departure, Ticks *waitin
return false;
}
static void ScheduledDispatchDepartureLocalFix(DepartureList *departure_list)
static void ScheduledDispatchDepartureLocalFix(DepartureList &departure_list)
{
/* Separate departure by each shared order group */
btree::btree_map<uint32_t, std::vector<Departure*>> separated_departure;
for (Departure* departure : *departure_list) {
separated_departure[departure->vehicle->orders->index].push_back(departure);
btree::btree_map<uint32_t, std::vector<Departure *>> separated_departure;
for (auto &departure : departure_list) {
separated_departure[departure->vehicle->orders->index].push_back(departure.get());
}
for (auto& pair : separated_departure) {
@ -255,7 +255,7 @@ static void ScheduledDispatchDepartureLocalFix(DepartureList *departure_list)
}
/* Re-sort the departure list */
std::sort(departure_list->begin(), departure_list->end(), [](Departure * const &a, Departure * const &b) -> bool {
std::sort(departure_list.begin(), departure_list.end(), [](std::unique_ptr<Departure> &a, std::unique_ptr<Departure> &b) -> bool {
return a->scheduled_tick < b->scheduled_tick;
});
}
@ -270,7 +270,7 @@ static void ScheduledDispatchDepartureLocalFix(DepartureList *departure_list)
* @param show_freight whether to include freight vehicles
* @return a list of departures, which is empty if an error occurred
*/
DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehicle *> &vehicles, DepartureType type, bool show_vehicles_via, bool show_pax, bool show_freight)
DepartureList MakeDepartureList(StationID station, const std::vector<const Vehicle *> &vehicles, DepartureType type, bool show_vehicles_via, bool show_pax, bool show_freight)
{
/* This function is the meat of the departure boards functionality. */
/* As an overview, it works by repeatedly considering the best possible next departure to show. */
@ -279,13 +279,13 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
/* This code can probably be made more efficient. I haven't done so in order to keep both its (relative) simplicity and my (relative) sanity. */
/* Having written that, it's not exactly slow at the moment. */
/* The list of departures which will be returned as a result. */
std::vector<Departure*> *result = new std::vector<Departure*>();
if (!show_pax && !show_freight) return {};
if (!show_pax && !show_freight) return result;
/* The list of departures which will be returned as a result. */
std::vector<std::unique_ptr<Departure>> result;
/* A list of the next scheduled orders to be considered for inclusion in the departure list. */
std::vector<OrderDate*> next_orders;
std::vector<std::unique_ptr<OrderDate>> next_orders;
/* The maximum possible date for departures to be scheduled to occur. */
const Ticks max_ticks = GetDeparturesMaxTicksAhead();
@ -426,7 +426,7 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
break;
}
OrderDate *od = new OrderDate();
std::unique_ptr<OrderDate> od = std::make_unique<OrderDate>();
od->order = order;
od->v = v;
/* We store the expected date for now, so that vehicles will be shown in order of expected time. */
@ -449,18 +449,18 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
/* Update least_order if this is the current least order. */
if (least_order == nullptr) {
least_order = od;
least_order = od.get();
} else if (type == D_ARRIVAL) {
if ((least_order->expected_tick - least_order->lateness - least_order->EffectiveWaitingTime()) > (od->expected_tick - od->lateness - od->EffectiveWaitingTime())) {
least_order = od;
least_order = od.get();
}
} else {
if ((least_order->expected_tick - least_order->lateness) > (od->expected_tick - od->lateness)) {
least_order = od;
least_order = od.get();
}
}
next_orders.push_back(od);
next_orders.push_back(std::move(od));
/* We're done with this vehicle. */
break;
@ -494,13 +494,14 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
/* least_order is the best candidate for the next departure. */
/* First, we check if we can stop looking for departures yet. */
if (result->size() >= _settings_client.gui.max_departures ||
if (result.size() >= _settings_client.gui.max_departures ||
least_order->expected_tick - least_order->lateness > max_ticks) {
break;
}
/* We already know the least order and that it's a suitable departure, so make it into a departure. */
Departure *d = new Departure();
std::unique_ptr<Departure> departure_ptr = std::make_unique<Departure>();
Departure *d = departure_ptr.get();
d->scheduled_tick = state_ticks_base + least_order->expected_tick - least_order->lateness;
d->lateness = least_order->lateness;
d->status = least_order->status;
@ -675,8 +676,8 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
bool duplicate = false;
if (_settings_client.gui.departure_merge_identical) {
for (uint i = 0; i < result->size(); ++i) {
if (*d == *((*result)[i])) {
for (uint i = 0; i < result.size(); ++i) {
if (*d == *(result[i])) {
duplicate = true;
break;
}
@ -684,11 +685,11 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
}
if (!duplicate) {
result->push_back(d);
result.push_back(std::move(departure_ptr));
if (_settings_client.gui.departure_smart_terminus && type == D_DEPARTURE) {
for (uint i = 0; i < result->size() - 1; ++i) {
Departure *d_first = (*result)[i];
for (uint i = 0; i < result.size() - 1; ++i) {
Departure *d_first = result[i].get();
uint k = (uint)d_first->calling_at.size() - 2;
uint j = (uint)d->calling_at.size();
while (j > 0) {
@ -723,8 +724,7 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
/* If the vehicle is expected to be late, we want to know what time it will arrive rather than depart. */
/* This is done because it looked silly to me to have a vehicle not be expected for another few days, yet it be at the same time pulling into the station. */
if (d->status != D_ARRIVED &&
d->lateness > 0) {
if (d->status != D_ARRIVED && d->lateness > 0) {
d->lateness -= least_order->order->GetWaitTime();
}
}
@ -799,8 +799,8 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
bool duplicate = false;
if (_settings_client.gui.departure_merge_identical) {
for (uint i = 0; i < result->size(); ++i) {
if (*d == *((*result)[i])) {
for (uint i = 0; i < result.size(); ++i) {
if (*d == *(result[i])) {
duplicate = true;
break;
}
@ -808,7 +808,7 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
}
if (!duplicate) {
result->push_back(d);
result.push_back(std::move(departure_ptr));
}
}
}
@ -906,7 +906,7 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
/* Find the new least order. */
for (uint i = 0; i < next_orders.size(); ++i) {
OrderDate *od = next_orders[i];
OrderDate *od = next_orders[i].get();
Ticks lod = least_order->expected_tick - least_order->lateness;
Ticks odd = od->expected_tick - od->lateness;
@ -922,12 +922,6 @@ DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehi
}
}
/* Avoid leaking OrderDate structs */
for (uint i = 0; i < next_orders.size(); ++i) {
OrderDate *od = next_orders[i];
delete od;
}
if (type == D_DEPARTURE) ScheduledDispatchDepartureLocalFix(result);
/* Done. Phew! */

2
src/departures_func.h
View File

@ -17,7 +17,7 @@
#include <vector>
DepartureList* MakeDepartureList(StationID station, const std::vector<const Vehicle *> &vehicles, DepartureType type = D_DEPARTURE,
DepartureList MakeDepartureList(StationID station, const std::vector<const Vehicle *> &vehicles, DepartureType type = D_DEPARTURE,
bool show_vehicles_via = false, bool show_pax = true, bool show_freight = true);
Ticks GetDeparturesMaxTicksAhead();

76
src/departures_gui.cpp
View File

@ -90,8 +90,8 @@ template<bool Twaypoint = false>
struct DeparturesWindow : public Window {
protected:
StationID station; ///< The station whose departures we're showing.
DepartureList *departures; ///< The current list of departures from this station.
DepartureList *arrivals; ///< The current list of arrivals from this station.
DepartureList departures; ///< The current list of departures from this station.
DepartureList arrivals; ///< The current list of arrivals from this station.
bool departures_invalid; ///< The departures and arrivals list are currently invalid.
bool vehicles_invalid; ///< The vehicles list is currently invalid.
uint entry_height; ///< The height of an entry in the departures list.
@ -114,7 +114,6 @@ protected:
virtual uint GetMinWidth() const;
static void RecomputeDateWidth();
virtual void DrawDeparturesListItems(const Rect &r) const;
void DeleteDeparturesList(DepartureList* list);
void ToggleCargoFilter(WidgetID widget, bool &flag)
{
@ -236,8 +235,6 @@ public:
DeparturesWindow(WindowDesc *desc, WindowNumber window_number) : Window(desc),
station(window_number),
departures(new DepartureList()),
arrivals(new DepartureList()),
departures_invalid(true),
vehicles_invalid(true),
elapsed_ms(0),
@ -289,12 +286,6 @@ public:
if (_pause_mode != PM_UNPAUSED) this->OnGameTick();
}
virtual ~DeparturesWindow()
{
this->DeleteDeparturesList(this->departures);
this->DeleteDeparturesList(this->arrivals);
}
void SetupValues()
{
this->entry_height = 1 + GetCharacterHeight(FS_NORMAL) + 1 + (_settings_client.gui.departure_larger_font ? GetCharacterHeight(FS_NORMAL) : GetCharacterHeight(FS_SMALL)) + 1 + 1;
@ -409,22 +400,22 @@ public:
id_v += (uint32_t)this->vscroll->GetPosition();
if (id_v >= (this->departures->size() + this->arrivals->size())) return; // click out of list bound
if (id_v >= (this->departures.size() + this->arrivals.size())) return; // click out of list bound
uint departure = 0;
uint arrival = 0;
/* Draw each departure. */
for (uint i = 0; i <= id_v; ++i) {
const Departure *d;
const Departure *d = nullptr;
if (arrival == this->arrivals->size()) {
d = (*(this->departures))[departure++];
} else if (departure == this->departures->size()) {
d = (*(this->arrivals))[arrival++];
if (arrival == this->arrivals.size()) {
d = this->departures[departure++].get();
} else if (departure == this->departures.size()) {
d = this->arrivals[arrival++].get();
} else {
d = (*(this->departures))[departure];
const Departure *a = (*(this->arrivals))[arrival];
d = this->departures[departure].get();
const Departure *a = this->arrivals[arrival].get();
if (a->scheduled_tick < d->scheduled_tick) {
d = a;
@ -492,12 +483,18 @@ public:
/* Recompute the list of departures if we're due to. */
if (this->calc_tick_countdown <= 0) {
this->calc_tick_countdown = _settings_client.gui.departure_calc_frequency;
this->DeleteDeparturesList(this->departures);
this->DeleteDeparturesList(this->arrivals);
bool show_pax = _settings_client.gui.departure_only_passengers ? true : this->show_pax;
bool show_freight = _settings_client.gui.departure_only_passengers ? false : this->show_freight;
this->departures = (this->departure_types[0] || _settings_client.gui.departure_show_both ? MakeDepartureList(this->station, this->vehicles, D_DEPARTURE, Twaypoint || this->departure_types[2], show_pax, show_freight) : new DepartureList());
this->arrivals = (this->departure_types[1] && !_settings_client.gui.departure_show_both ? MakeDepartureList(this->station, this->vehicles, D_ARRIVAL, false, show_pax, show_freight) : new DepartureList());
if (this->departure_types[0] || _settings_client.gui.departure_show_both) {
this->departures = MakeDepartureList(this->station, this->vehicles, D_DEPARTURE, Twaypoint || this->departure_types[2], show_pax, show_freight);
} else {
this->departures.clear();
}
if (this->departure_types[1] && !_settings_client.gui.departure_show_both) {
this->arrivals = MakeDepartureList(this->station, this->vehicles, D_ARRIVAL, false, show_pax, show_freight);
} else {
this->arrivals.clear();
}
this->departures_invalid = false;
this->SetWidgetDirty(WID_DB_LIST);
}
@ -530,7 +527,7 @@ public:
virtual void OnPaint() override
{
this->vscroll->SetCount(std::min<uint>(_settings_client.gui.max_departures, (uint)this->departures->size() + (uint)this->arrivals->size()));
this->vscroll->SetCount(std::min<uint>(_settings_client.gui.max_departures, (uint)this->departures.size() + (uint)this->arrivals.size()));
this->DrawWidgets();
}
@ -640,23 +637,6 @@ uint DeparturesWindow<Twaypoint>::GetMinWidth() const
return result + ScaleGUITrad(140);
}
/**
* Deletes this window's departure list.
*/
template<bool Twaypoint>
void DeparturesWindow<Twaypoint>::DeleteDeparturesList(DepartureList *list)
{
/* SmallVector uses free rather than delete on its contents (which doesn't invoke the destructor), so we need to delete each departure manually. */
for (uint i = 0; i < list->size(); ++i) {
Departure **d = &(*list)[i];
delete *d;
/* Make sure a double free doesn't happen. */
*d = nullptr;
}
delete list;
list = nullptr;
}
/**
* Draws a list of departures.
*/
@ -676,7 +656,7 @@ void DeparturesWindow<Twaypoint>::DrawDeparturesListItems(const Rect &r) const
const int text_right = right - (rtl ? text_offset : 0);
int y = r.top + 1;
uint max_departures = std::min<uint>(this->vscroll->GetPosition() + this->vscroll->GetCapacity(), (uint)this->departures->size() + (uint)this->arrivals->size());
uint max_departures = std::min<uint>(this->vscroll->GetPosition() + this->vscroll->GetCapacity(), (uint)this->departures.size() + (uint)this->arrivals.size());
if (max_departures > _settings_client.gui.max_departures) {
max_departures = _settings_client.gui.max_departures;
@ -755,13 +735,13 @@ void DeparturesWindow<Twaypoint>::DrawDeparturesListItems(const Rect &r) const
for (uint i = 0; i < max_departures; ++i) {
const Departure *d;
if (arrival == this->arrivals->size()) {
d = (*(this->departures))[departure++];
} else if (departure == this->departures->size()) {
d = (*(this->arrivals))[arrival++];
if (arrival == this->arrivals.size()) {
d = this->departures[departure++].get();
} else if (departure == this->departures.size()) {
d = this->arrivals[arrival++].get();
} else {
d = (*(this->departures))[departure];
const Departure *a = (*(this->arrivals))[arrival];
d = this->departures[departure].get();
const Departure *a = this->arrivals[arrival].get();
if (a->scheduled_tick < d->scheduled_tick) {
d = a;

2
src/departures_type.h
View File

@ -113,6 +113,6 @@ struct Departure {
}
};
typedef std::vector<Departure*> DepartureList;
typedef std::vector<std::unique_ptr<Departure>> DepartureList;
#endif /* DEPARTURES_TYPE_H */