core: replace divesite_table_t by a vector of std::unique_ptr<>s

This is a long commit, because it introduces a new abstraction:
a general std::vector<> of std::unique_ptrs<>.

Moreover, it replaces a number of pointers by C++ references,
when the callee does not suppoert null objects.

This simplifies memory management and makes ownership more
explicit. It is a proof-of-concept and a test-bed for
the other core data structrures.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

backend-shared/exportfuncs.cpp

@@ -325,21 +325,18 @@ std::vector<const dive_site *> getDiveSitesToExport(bool selectedOnly)
 		return res;
 	}
 
-	res.reserve(divelog.sites->nr);
+	res.reserve(divelog.sites->size());
-	for (int i = 0; i < divelog.sites->nr; i++) {
+	for (const auto &ds: *divelog.sites) {
-		struct dive_site *ds = get_dive_site(i, divelog.sites);
+		if (dive_site_is_empty(ds.get()))
-		if (dive_site_is_empty(ds))
 			continue;
 		if (selectedOnly && !is_dive_site_selected(*ds))
 			continue;
-		res.push_back(ds);
+		res.push_back(ds.get());
 	}
 #else
 	/* walk the dive site list */
-	int i;
+	for (const auto &ds: *divelog.sites)
-	const struct dive_site *ds;
+		res.push_back(ds.get());
-	for_each_dive_site (i, ds, divelog.sites)
-		res.push_back(get_dive_site(i, divelog.sites));
 #endif
 	return res;
 }

commands/command.cpp

@@ -134,9 +134,9 @@ void addDiveSite(const QString &name)
 	execute(new AddDiveSite(name));
 }
 
-void importDiveSites(struct dive_site_table *sites, const QString &source)
+void importDiveSites(dive_site_table sites, const QString &source)
 {
-	execute(new ImportDiveSites(sites, source));
+	execute(new ImportDiveSites(std::move(sites), source));
 }
 
 void mergeDiveSites(dive_site *ds, const QVector<dive_site *> &sites)

commands/command.h

@@ -68,7 +68,7 @@ void editDiveSiteCountry(dive_site *ds, const QString &value);
 void editDiveSiteLocation(dive_site *ds, location_t value);
 void editDiveSiteTaxonomy(dive_site *ds, taxonomy_data &value); // value is consumed (i.e. will be erased after call)!
 void addDiveSite(const QString &name);
-void importDiveSites(struct dive_site_table *sites, const QString &source);
+void importDiveSites(dive_site_table sites, const QString &source); // takes ownership of dive site table
 void mergeDiveSites(dive_site *ds, const QVector<dive_site *> &sites);
 void purgeUnusedDiveSites();
 

commands/command_divelist.cpp

@@ -142,9 +142,9 @@ DivesAndTripsToAdd DiveListBase::removeDives(DivesAndSitesToRemove &divesAndSite
 	divesAndSitesToDelete.dives.clear();
 
 	for (dive_site *ds: divesAndSitesToDelete.sites) {
-		int idx = unregister_dive_site(ds);
+		auto res = divelog.sites->pull(ds);
-		sitesToAdd.emplace_back(ds);
+		sitesToAdd.push_back(std::move(res.ptr));
-		emit diveListNotifier.diveSiteDeleted(ds, idx);
+		emit diveListNotifier.diveSiteDeleted(ds, res.idx);
 	}
 	divesAndSitesToDelete.sites.clear();
 
@@ -217,9 +217,9 @@ DivesAndSitesToRemove DiveListBase::addDives(DivesAndTripsToAdd &toAdd)
 
 	// Finally, add any necessary dive sites
 	for (std::unique_ptr<dive_site> &ds: toAdd.sites) {
-		sites.push_back(ds.get());
+		auto res = divelog.sites->register_site(std::move(ds));
-		int idx = register_dive_site(ds.release()); // Return ownership to backend
+		sites.push_back(res.ptr);
-		emit diveListNotifier.diveSiteAdded(sites.back(), idx);
+		emit diveListNotifier.diveSiteAdded(sites.back(), res.idx);
 	}
 	toAdd.sites.clear();
 
@@ -478,10 +478,10 @@ ImportDives::ImportDives(struct divelog *log, int flags, const QString &source)
 	struct dive_table dives_to_add = empty_dive_table;
 	struct dive_table dives_to_remove = empty_dive_table;
 	struct trip_table trips_to_add = empty_trip_table;
-	struct dive_site_table sites_to_add = empty_dive_site_table;
+	dive_site_table sites_to_add;
 	process_imported_dives(log, flags,
 			       &dives_to_add, &dives_to_remove, &trips_to_add,
-			       &sites_to_add, &devicesToAddAndRemove);
+			       sites_to_add, &devicesToAddAndRemove);
 
 	// Add trips to the divesToAdd.trips structure
 	divesToAdd.trips.reserve(trips_to_add.nr);
@@ -489,9 +489,7 @@ ImportDives::ImportDives(struct divelog *log, int flags, const QString &source)
 		divesToAdd.trips.emplace_back(trips_to_add.trips[i]);
 
 	// Add sites to the divesToAdd.sites structure
-	divesToAdd.sites.reserve(sites_to_add.nr);
+	divesToAdd.sites = std::move(sites_to_add);
-	for (int i = 0; i < sites_to_add.nr; ++i)
-		divesToAdd.sites.emplace_back(sites_to_add.dive_sites[i]);
 
 	// Add dives to the divesToAdd.dives structure
 	divesToAdd.dives.reserve(dives_to_add.nr);
@@ -525,7 +523,6 @@ ImportDives::ImportDives(struct divelog *log, int flags, const QString &source)
 	free(dives_to_add.dives);
 	free(dives_to_remove.dives);
 	free(trips_to_add.trips);
-	free(sites_to_add.dive_sites);
 }
 
 bool ImportDives::workToBeDone()

commands/command_divesite.cpp

@@ -30,9 +30,9 @@ static std::vector<dive_site *> addDiveSites(std::vector<std::unique_ptr<dive_si
 		}
 
 		// Add dive site to core, but remember a non-owning pointer first.
-		res.push_back(ds.get());
+		auto add_res = divelog.sites->put(std::move(ds)); // Return ownership to backend.
-		int idx = register_dive_site(ds.release()); // Return ownership to backend.
+		res.push_back(add_res.ptr);
-		emit diveListNotifier.diveSiteAdded(res.back(), idx); // Inform frontend of new dive site.
+		emit diveListNotifier.diveSiteAdded(res.back(), add_res.idx); // Inform frontend of new dive site.
 	}
 
 	emit diveListNotifier.divesChanged(changedDives, DiveField::DIVESITE);
@@ -60,9 +60,9 @@ static std::vector<std::unique_ptr<dive_site>> removeDiveSites(std::vector<dive_
 		}
 
 		// Remove dive site from core and take ownership.
-		int idx = unregister_dive_site(ds);
+		auto pull_res = divelog.sites->pull(ds);
-		res.emplace_back(ds);
+		res.push_back(std::move(pull_res.ptr));
-		emit diveListNotifier.diveSiteDeleted(ds, idx); // Inform frontend of removed dive site.
+		emit diveListNotifier.diveSiteDeleted(ds, pull_res.idx); // Inform frontend of removed dive site.
 	}
 
 	emit diveListNotifier.divesChanged(changedDives, DiveField::DIVESITE);
@@ -94,25 +94,18 @@ void AddDiveSite::undo()
 	sitesToAdd = removeDiveSites(sitesToRemove);
 }
 
-ImportDiveSites::ImportDiveSites(struct dive_site_table *sites, const QString &source)
+ImportDiveSites::ImportDiveSites(dive_site_table sites, const QString &source)
 {
 	setText(Command::Base::tr("import dive sites from %1").arg(source));
 
-	for (int i = 0; i < sites->nr; ++i) {
+	for (auto &new_ds: sites) {
-		struct dive_site *new_ds = sites->dive_sites[i];
-
 		// Don't import dive sites that already exist. Currently we only check for
 		// the same name. We might want to be smarter here and merge dive site data, etc.
-		struct dive_site *old_ds = get_same_dive_site(new_ds);
+		struct dive_site *old_ds = get_same_dive_site(*new_ds);
-		if (old_ds) {
+		if (old_ds)
-			delete new_ds;
 			continue;
-		}
+		sitesToAdd.push_back(std::move(new_ds));
-		sitesToAdd.emplace_back(new_ds);
 	}
-
-	// All site have been consumed
-	sites->nr = 0;
 }
 
 bool ImportDiveSites::workToBeDone()
@@ -153,10 +146,9 @@ void DeleteDiveSites::undo()
 PurgeUnusedDiveSites::PurgeUnusedDiveSites()
 {
 	setText(Command::Base::tr("purge unused dive sites"));
-	for (int i = 0; i < divelog.sites->nr; ++i) {
+	for (const auto &ds: *divelog.sites) {
-		dive_site *ds = divelog.sites->dive_sites[i];
 		if (ds->dives.empty())
-			sitesToRemove.push_back(ds);
+			sitesToRemove.push_back(ds.get());
 	}
 }
 
@@ -391,7 +383,7 @@ ApplyGPSFixes::ApplyGPSFixes(const std::vector<DiveAndLocation> &fixes)
 				siteLocations.push_back({ ds, dl.location });
 			}
 		} else {
-			ds = create_dive_site(dl.name.toStdString(), divelog.sites);
+			ds = create_dive_site(dl.name.toStdString(), *divelog.sites);
 			ds->location = dl.location;
 			add_dive_to_dive_site(dl.d, ds);
 			dl.d->dive_site = nullptr; // This will be set on redo()

commands/command_divesite.h

@@ -36,8 +36,8 @@ private:
 
 class ImportDiveSites : public Base {
 public:
-	// Note: the dive site table is consumed after the call it will be empty.
+	// Note: Takes ownership of dive site table
-	ImportDiveSites(struct dive_site_table *sites, const QString &source);
+	ImportDiveSites(dive_site_table sites, const QString &source);
 private:
 	bool workToBeDone() override;
 	void undo() override;

commands/command_edit.cpp

@@ -401,17 +401,17 @@ EditDiveSiteNew::EditDiveSiteNew(const QString &newName, bool currentDiveOnly) :
 void EditDiveSiteNew::undo()
 {
 	EditDiveSite::undo();
-	int idx = unregister_dive_site(diveSiteToRemove);
+	auto res = divelog.sites->pull(diveSiteToRemove);
-	diveSiteToAdd.reset(diveSiteToRemove);
+	diveSiteToAdd = std::move(res.ptr);
-	emit diveListNotifier.diveSiteDeleted(diveSiteToRemove, idx); // Inform frontend of removed dive site.
+	emit diveListNotifier.diveSiteDeleted(diveSiteToRemove, res.idx); // Inform frontend of removed dive site.
 	diveSiteToRemove = nullptr;
 }
 
 void EditDiveSiteNew::redo()
 {
-	diveSiteToRemove = diveSiteToAdd.get();
+	auto res = divelog.sites->register_site(std::move(diveSiteToAdd)); // Return ownership to backend.
-	int idx = register_dive_site(diveSiteToAdd.release()); // Return ownership to backend.
+	diveSiteToRemove = res.ptr;
-	emit diveListNotifier.diveSiteAdded(diveSiteToRemove, idx); // Inform frontend of new dive site.
+	emit diveListNotifier.diveSiteAdded(diveSiteToRemove, res.idx); // Inform frontend of new dive site.
 	EditDiveSite::redo();
 }
 
@@ -1439,9 +1439,9 @@ EditDive::EditDive(dive *oldDiveIn, dive *newDiveIn, dive_site *createDs, dive_s
 void EditDive::undo()
 {
 	if (siteToRemove) {
-		int idx = unregister_dive_site(siteToRemove);
+		auto res = divelog.sites->pull(siteToRemove);
-		siteToAdd.reset(siteToRemove);
+		siteToAdd = std::move(res.ptr);
-		emit diveListNotifier.diveSiteDeleted(siteToRemove, idx); // Inform frontend of removed dive site.
+		emit diveListNotifier.diveSiteDeleted(siteToRemove, res.idx); // Inform frontend of removed dive site.
 	}
 
 	exchangeDives();
@@ -1451,9 +1451,9 @@ void EditDive::undo()
 void EditDive::redo()
 {
 	if (siteToAdd) {
-		siteToRemove = siteToAdd.get();
+		auto res = divelog.sites->register_site(std::move(siteToAdd)); // Return ownership to backend.
-		int idx = register_dive_site(siteToAdd.release()); // Return ownership to backend.
+		siteToRemove = res.ptr;
-		emit diveListNotifier.diveSiteAdded(siteToRemove, idx); // Inform frontend of new dive site.
+		emit diveListNotifier.diveSiteAdded(siteToRemove, res.idx); // Inform frontend of new dive site.
 	}
 
 	exchangeDives();

commands/command_pictures.cpp

@@ -217,9 +217,9 @@ void AddPictures::undo()
 
 	// Remove dive sites
 	for (dive_site *siteToRemove: sitesToRemove) {
-		int idx = unregister_dive_site(siteToRemove);
+		auto res = divelog.sites->pull(siteToRemove);
-		sitesToAdd.emplace_back(siteToRemove);
+		sitesToAdd.push_back(std::move(res.ptr));
-		emit diveListNotifier.diveSiteDeleted(siteToRemove, idx); // Inform frontend of removed dive site.
+		emit diveListNotifier.diveSiteDeleted(siteToRemove, res.idx); // Inform frontend of removed dive site.
 	}
 	sitesToRemove.clear();
 }
@@ -228,9 +228,9 @@ void AddPictures::redo()
 {
 	// Add dive sites
 	for (std::unique_ptr<dive_site> &siteToAdd: sitesToAdd) {
-		sitesToRemove.push_back(siteToAdd.get());
+		auto res = divelog.sites->register_site(std::move(siteToAdd)); // Return ownership to backend.
-		int idx = register_dive_site(siteToAdd.release()); // Return ownership to backend.
+		sitesToRemove.push_back(res.ptr);
-		emit diveListNotifier.diveSiteAdded(sitesToRemove.back(), idx); // Inform frontend of new dive site.
+		emit diveListNotifier.diveSiteAdded(sitesToRemove.back(), res.idx); // Inform frontend of new dive site.
 	}
 	sitesToAdd.clear();
 

core/datatrak.cpp

@@ -211,9 +211,9 @@ static char *dt_dive_parser(unsigned char *runner, struct dive *dt_dive, struct
 	 */
 	{
 		std::string buffer2 = std::string((char *)locality) + " " + (char *)dive_point;
-		struct dive_site *ds = get_dive_site_by_name(buffer2, log->sites);
+		struct dive_site *ds = get_dive_site_by_name(buffer2, *log->sites);
 		if (!ds)
-			ds = create_dive_site(buffer2, log->sites);
+			ds = create_dive_site(buffer2, *log->sites);
 		add_dive_to_dive_site(dt_dive, ds);
 	}
 	free(locality);

core/dive.h

@@ -19,7 +19,6 @@ extern const char *divemode_text_ui[];
 extern const char *divemode_text[];
 
 struct dive_site;
-struct dive_site_table;
 struct dive_table;
 struct dive_trip;
 struct full_text_cache;

core/divelist.cpp

@@ -950,13 +950,13 @@ void add_imported_dives(struct divelog *import_log, int flags)
 	struct dive_table dives_to_add = empty_dive_table;
 	struct dive_table dives_to_remove = empty_dive_table;
 	struct trip_table trips_to_add = empty_trip_table;
-	struct dive_site_table dive_sites_to_add = empty_dive_site_table;
+	dive_site_table dive_sites_to_add;
 	struct device_table *devices_to_add = alloc_device_table();
 
 	/* Process imported dives and generate lists of dives
 	 * to-be-added and to-be-removed */
 	process_imported_dives(import_log, flags, &dives_to_add, &dives_to_remove, &trips_to_add,
-			       &dive_sites_to_add, devices_to_add);
+			       dive_sites_to_add, devices_to_add);
 
 	/* Start by deselecting all dives, so that we don't end up with an invalid selection */
 	select_single_dive(NULL);
@@ -990,9 +990,8 @@ void add_imported_dives(struct divelog *import_log, int flags)
 	trips_to_add.nr = 0;
 
 	/* Add new dive sites */
-	for (i = 0; i < dive_sites_to_add.nr; i++)
+	for (auto &ds: dive_sites_to_add)
-		register_dive_site(dive_sites_to_add.dive_sites[i]);
+		divelog.sites->register_site(std::move(ds));
-	dive_sites_to_add.nr = 0;
 
 	/* Add new devices */
 	for (i = 0; i < nr_devices(devices_to_add); i++) {
@@ -1008,7 +1007,6 @@ void add_imported_dives(struct divelog *import_log, int flags)
 	free(dives_to_add.dives);
 	free(dives_to_remove.dives);
 	free(trips_to_add.trips);
-	free(dive_sites_to_add.dive_sites);
 
 	/* Inform frontend of reset data. This should reset all the models. */
 	emit_reset_signal();
@@ -1083,7 +1081,7 @@ bool try_to_merge_trip(struct dive_trip *trip_import, struct dive_table *import_
 void process_imported_dives(struct divelog *import_log, int flags,
 			    /* output parameters: */
 			    struct dive_table *dives_to_add, struct dive_table *dives_to_remove,
-			    struct trip_table *trips_to_add, struct dive_site_table *sites_to_add,
+			    struct trip_table *trips_to_add, dive_site_table &sites_to_add,
 			    struct device_table *devices_to_add)
 {
 	int i, j, nr, start_renumbering_at = 0;
@@ -1096,7 +1094,7 @@ void process_imported_dives(struct divelog *import_log, int flags,
 	clear_dive_table(dives_to_add);
 	clear_dive_table(dives_to_remove);
 	clear_trip_table(trips_to_add);
-	clear_dive_site_table(sites_to_add);
+	sites_to_add.clear();
 	clear_device_table(devices_to_add);
 
 	/* Check if any of the new dives has a number. This will be
@@ -1130,36 +1128,33 @@ void process_imported_dives(struct divelog *import_log, int flags,
 		autogroup_dives(import_log->dives, import_log->trips);
 
 	/* If dive sites already exist, use the existing versions. */
-	for (i = 0; i  < import_log->sites->nr; i++) {
+	for (auto &new_ds: *import_log->sites) {
-		struct dive_site *new_ds = import_log->sites->dive_sites[i];
+		struct dive_site *old_ds = get_same_dive_site(*new_ds);
-		struct dive_site *old_ds = get_same_dive_site(new_ds);
 
 		/* Check if it dive site is actually used by new dives. */
 		for (j = 0; j < import_log->dives->nr; j++) {
-			if (import_log->dives->dives[j]->dive_site == new_ds)
+			if (import_log->dives->dives[j]->dive_site == new_ds.get())
 				break;
 		}
 
 		if (j == import_log->dives->nr) {
-			/* Dive site not even used - free it and go to next. */
+			/* Dive site not even used. */
-			delete new_ds;
 			continue;
 		}
 
 		if (!old_ds) {
 			/* Dive site doesn't exist. Add it to list of dive sites to be added. */
 			new_ds->dives.clear(); /* Caller is responsible for adding dives to site */
-			add_dive_site_to_table(new_ds, sites_to_add);
+			sites_to_add.put(std::move(new_ds));
-			continue;
+		} else {
+			/* Dive site already exists - use the old one. */
+			for (j = 0; j < import_log->dives->nr; j++) {
+				if (import_log->dives->dives[j]->dive_site == new_ds.get())
+					import_log->dives->dives[j]->dive_site = old_ds;
+			}
 		}
-		/* Dive site already exists - use the old and free the new. */
-		for (j = 0; j < import_log->dives->nr; j++) {
-			if (import_log->dives->dives[j]->dive_site == new_ds)
-				import_log->dives->dives[j]->dive_site = old_ds;
-		}
-		delete new_ds;
 	}
-	import_log->sites->nr = 0; /* All dive sites were consumed */
+	import_log->sites->clear();
 
 	/* Merge overlapping trips. Since both trip tables are sorted, we
 	 * could be smarter here, but realistically not a whole lot of trips

core/divelist.h

@@ -7,7 +7,7 @@
 struct dive;
 struct divelog;
 struct trip_table;
-struct dive_site_table;
+class dive_site_table;
 struct device_table;
 struct deco_state;
 
@@ -34,7 +34,7 @@ extern void process_loaded_dives();
 extern void add_imported_dives(struct divelog *log, int flags);
 extern void process_imported_dives(struct divelog *import_log, int flags,
 				   struct dive_table *dives_to_add, struct dive_table *dives_to_remove,
-				   struct trip_table *trips_to_add, struct dive_site_table *sites_to_add,
+				   struct trip_table *trips_to_add, dive_site_table &sites_to_add,
 				   struct device_table *devices_to_add);
 
 extern int dive_table_get_insertion_index(struct dive_table *table, struct dive *dive);

core/divelog.cpp

@@ -22,14 +22,12 @@ divelog::divelog() :
 {
 	*dives = empty_dive_table;
 	*trips = empty_trip_table;
-	*sites = empty_dive_site_table;
 }
 
 divelog::~divelog()
 {
 	clear_dive_table(dives);
 	clear_trip_table(trips);
-	clear_dive_site_table(sites);
 	delete dives;
 	delete trips;
 	delete sites;
@@ -40,16 +38,14 @@ divelog::~divelog()
 divelog::divelog(divelog &&log) :
 	dives(new dive_table),
 	trips(new trip_table),
-	sites(new dive_site_table),
+	sites(new dive_site_table(std::move(*log.sites))),
 	devices(new device_table),
 	filter_presets(new filter_preset_table)
 {
 	*dives = empty_dive_table;
 	*trips = empty_trip_table;
-	*sites = empty_dive_site_table;
 	move_dive_table(log.dives, dives);
 	move_trip_table(log.trips, trips);
-	move_dive_site_table(log.sites, sites);
 	*devices = std::move(*log.devices);
 	*filter_presets = std::move(*log.filter_presets);
 }
@@ -58,7 +54,7 @@ struct divelog &divelog::operator=(divelog &&log)
 {
 	move_dive_table(log.dives, dives);
 	move_trip_table(log.trips, trips);
-	move_dive_site_table(log.sites, sites);
+	*sites = std::move(*log.sites);
 	*devices = std::move(*log.devices);
 	*filter_presets = std::move(*log.filter_presets);
 	return *this;
@@ -82,8 +78,7 @@ void divelog::clear()
 {
 	while (dives->nr > 0)
 		delete_single_dive(this, dives->nr - 1);
-	while (sites->nr)
+	sites->clear();
-		delete_dive_site(get_dive_site(0, sites), sites);
 	if (trips->nr != 0) {
 		report_info("Warning: trip table not empty in divelog::clear()!");
 		trips->nr = 0;

core/divelog.h

@@ -5,7 +5,7 @@
 
 struct dive_table;
 struct trip_table;
-struct dive_site_table;
+class dive_site_table;
 struct device_table;
 struct filter_preset_table;
 
@@ -14,7 +14,7 @@ struct filter_preset_table;
 struct divelog {
 	struct dive_table *dives;
 	struct trip_table *trips;
-	struct dive_site_table *sites;
+	dive_site_table *sites;
 	struct device_table *devices;
 	struct filter_preset_table *filter_presets;
 	bool autogroup;

core/divesite.cpp

@@ -10,71 +10,53 @@
 #include "membuffer.h"
 #include "pref.h"
 #include "subsurface-string.h"
-#include "table.h"
 #include "sha1.h"
 
 #include <math.h>
 
-int get_divesite_idx(const struct dive_site *ds, struct dive_site_table *ds_table)
+int get_divesite_idx(const struct dive_site *ds, dive_site_table &ds_table)
 {
-	int i;
+	auto it = std::find_if(ds_table.begin(), ds_table.end(), [ds] (const auto &ds2) { return ds2.get() == ds; });
-	const struct dive_site *d;
+	return it != ds_table.end() ? it - ds_table.begin() : -1;
-	// tempting as it may be, don't die when called with ds=NULL
-	if (ds)
-		for_each_dive_site(i, d, ds_table) {
-			if (d == ds)
-				return i;
-		}
-	return -1;
 }
 
-// TODO: keep table sorted by UUID and do a binary search?
+template <typename PRED>
-struct dive_site *get_dive_site_by_uuid(uint32_t uuid, struct dive_site_table *ds_table)
+struct dive_site *get_dive_site_by_predicate(dive_site_table &ds_table, PRED pred)
 {
-	int i;
+	auto it = std::find_if(ds_table.begin(), ds_table.end(), pred);
-	struct dive_site *ds;
+	return it != ds_table.end() ? it->get() : NULL;
-	for_each_dive_site (i, ds, ds_table)
+}
-		if (ds->uuid == uuid)
+
-			return get_dive_site(i, ds_table);
+struct dive_site *get_dive_site_by_uuid(uint32_t uuid, dive_site_table &ds_table)
-	return NULL;
+{
+	// The table is sorted by uuid
+	auto it = std::lower_bound(ds_table.begin(), ds_table.end(), uuid,
+				   [] (const auto &ds, auto uuid) { return ds->uuid < uuid; });
+	return it != ds_table.end() && (*it)->uuid == uuid ? it->get() : NULL;
 }
 
 /* there could be multiple sites of the same name - return the first one */
-struct dive_site *get_dive_site_by_name(const std::string &name, struct dive_site_table *ds_table)
+struct dive_site *get_dive_site_by_name(const std::string &name, dive_site_table &ds_table)
 {
-	int i;
+	return get_dive_site_by_predicate(ds_table,
-	struct dive_site *ds;
+			       [&name](const auto &ds) { return ds->name == name; });
-	for_each_dive_site (i, ds, ds_table) {
-		if (ds->name == name)
-			return ds;
-	}
-	return NULL;
 }
 
 /* there could be multiple sites at the same GPS fix - return the first one */
-struct dive_site *get_dive_site_by_gps(const location_t *loc, struct dive_site_table *ds_table)
+struct dive_site *get_dive_site_by_gps(const location_t *loc, dive_site_table &ds_table)
 {
-	int i;
+	return get_dive_site_by_predicate(ds_table,
-	struct dive_site *ds;
+			       [loc](const auto &ds) { return ds->location == *loc; });
-	for_each_dive_site (i, ds, ds_table) {
-		if (*loc == ds->location)
-			return ds;
-	}
-	return NULL;
 }
 
 /* to avoid a bug where we have two dive sites with different name and the same GPS coordinates
  * and first get the gps coordinates (reading a V2 file) and happen to get back "the other" name,
  * this function allows us to verify if a very specific name/GPS combination already exists */
-struct dive_site *get_dive_site_by_gps_and_name(const std::string &name, const location_t *loc, struct dive_site_table *ds_table)
+struct dive_site *get_dive_site_by_gps_and_name(const std::string &name, const location_t *loc, dive_site_table &ds_table)
 {
-	int i;
+	return get_dive_site_by_predicate(ds_table,
-	struct dive_site *ds;
+			       [&name, loc](const auto &ds) { return ds->location == *loc &&
-	for_each_dive_site (i, ds, ds_table) {
+			       					     ds->name == name; });
-		if (*loc == ds->location && ds->name == name)
-			return ds;
-	}
-	return NULL;
 }
 
 // Calculate the distance in meters between two coordinates.
@@ -96,52 +78,21 @@ unsigned int get_distance(const location_t *loc1, const location_t *loc2)
 }
 
 /* find the closest one, no more than distance meters away - if more than one at same distance, pick the first */
-struct dive_site *get_dive_site_by_gps_proximity(const location_t *loc, int distance, struct dive_site_table *ds_table)
+struct dive_site *get_dive_site_by_gps_proximity(const location_t *loc, int distance, dive_site_table &ds_table)
 {
-	int i;
+	struct dive_site *res = nullptr;
-	struct dive_site *ds, *res = NULL;
 	unsigned int cur_distance, min_distance = distance;
-	for_each_dive_site (i, ds, ds_table) {
+	for (const auto &ds: ds_table) {
-		if (dive_site_has_gps_location(ds) &&
+		if (dive_site_has_gps_location(ds.get()) &&
 		    (cur_distance = get_distance(&ds->location, loc)) < min_distance) {
 			min_distance = cur_distance;
-			res = ds;
+			res = ds.get();
 		}
 	}
 	return res;
 }
 
-int register_dive_site(struct dive_site *ds)
+dive_site_table::put_result dive_site_table::register_site(std::unique_ptr<dive_site> ds)
-{
-	return add_dive_site_to_table(ds, divelog.sites);
-}
-
-static int compare_sites(const struct dive_site *a, const struct dive_site *b)
-{
-	return a->uuid > b->uuid ? 1 : a->uuid == b->uuid ? 0 : -1;
-}
-
-static int site_less_than(const struct dive_site *a, const struct dive_site *b)
-{
-	return compare_sites(a, b) < 0;
-}
-
-static void free_dive_site(struct dive_site *ds)
-{
-	delete ds;
-}
-
-static MAKE_GROW_TABLE(dive_site_table, struct dive_site *, dive_sites)
-static MAKE_GET_INSERTION_INDEX(dive_site_table, struct dive_site *, dive_sites, site_less_than)
-static MAKE_ADD_TO(dive_site_table, struct dive_site *, dive_sites)
-static MAKE_REMOVE_FROM(dive_site_table, dive_sites)
-static MAKE_GET_IDX(dive_site_table, struct dive_site *, dive_sites)
-MAKE_SORT(dive_site_table, struct dive_site *, dive_sites, compare_sites)
-static MAKE_REMOVE(dive_site_table, struct dive_site *, dive_site)
-MAKE_CLEAR_TABLE(dive_site_table, dive_sites, dive_site)
-MAKE_MOVE_TABLE(dive_site_table, dive_sites)
-
-int add_dive_site_to_table(struct dive_site *ds, struct dive_site_table *ds_table)
 {
 	/* If the site doesn't yet have an UUID, create a new one.
 	 * Make this deterministic for testing. */
@@ -158,12 +109,10 @@ int add_dive_site_to_table(struct dive_site *ds, struct dive_site_table *ds_tabl
 
 	/* Take care to never have the same uuid twice. This could happen on
 	 * reimport of a log where the dive sites have diverged */
-	while (ds->uuid == 0 || get_dive_site_by_uuid(ds->uuid, ds_table) != NULL)
+	while (ds->uuid == 0 || get_dive_site_by_uuid(ds->uuid, *this) != NULL)
 		++ds->uuid;
 
-	int idx = dive_site_table_get_insertion_index(ds_table, ds);
+	return put(std::move(ds));
-	add_to_dive_site_table(ds_table, idx, ds);
-	return idx;
 }
 
 dive_site::dive_site()
@@ -178,24 +127,23 @@ dive_site::dive_site(const std::string &name, const location_t *loc) : name(name
 {
 }
 
+dive_site::dive_site(uint32_t uuid) : uuid(uuid)
+{
+}
+
 dive_site::~dive_site()
 {
 }
 
 /* when parsing, dive sites are identified by uuid */
-struct dive_site *alloc_or_get_dive_site(uint32_t uuid, struct dive_site_table *ds_table)
+struct dive_site *alloc_or_get_dive_site(uint32_t uuid, dive_site_table &ds_table)
 {
 	struct dive_site *ds;
 
 	if (uuid && (ds = get_dive_site_by_uuid(uuid, ds_table)) != NULL)
 		return ds;
 
-	ds = new dive_site;
+	return ds_table.register_site(std::make_unique<dive_site>(uuid)).ptr;
-	ds->uuid = uuid;
-
-	add_dive_site_to_table(ds, ds_table);
-
-	return ds;
 }
 
 size_t nr_of_dives_at_dive_site(const dive_site &ds)
@@ -209,33 +157,16 @@ bool is_dive_site_selected(const struct dive_site &ds)
 		      [](dive *dive) { return dive->selected; });
 }
 
-int unregister_dive_site(struct dive_site *ds)
-{
-	return remove_dive_site(ds, divelog.sites);
-}
-
-void delete_dive_site(struct dive_site *ds, struct dive_site_table *ds_table)
-{
-	if (!ds)
-		return;
-	remove_dive_site(ds, ds_table);
-	delete ds;
-}
-
 /* allocate a new site and add it to the table */
-struct dive_site *create_dive_site(const std::string &name, struct dive_site_table *ds_table)
+struct dive_site *create_dive_site(const std::string &name, dive_site_table &ds_table)
 {
-	struct dive_site *ds = new dive_site(name);
+	return ds_table.register_site(std::make_unique<dive_site>(name)).ptr;
-	add_dive_site_to_table(ds, ds_table);
-	return ds;
 }
 
 /* same as before, but with GPS data */
-struct dive_site *create_dive_site_with_gps(const std::string &name, const location_t *loc, struct dive_site_table *ds_table)
+struct dive_site *create_dive_site_with_gps(const std::string &name, const location_t *loc, dive_site_table &ds_table)
 {
-	struct dive_site *ds = new dive_site(name, loc);
+	return ds_table.register_site(std::make_unique<dive_site>(name, loc)).ptr;
-	add_dive_site_to_table(ds, ds_table);
-	return ds;
 }
 
 /* if all fields are empty, the dive site is pointless */
@@ -270,22 +201,18 @@ static void merge_string(std::string &a, const std::string &b)
  * Taxonomy is not compared, as no taxonomy is generated on
  * import.
  */
-static bool same_dive_site(const struct dive_site *a, const struct dive_site *b)
+static bool same_dive_site(const struct dive_site &a, const struct dive_site &b)
 {
-	return a->name == b->name
+	return a.name == b.name
-	    && a->location == b->location
+	    && a.location == b.location
-	    && a->description == b->description
+	    && a.description == b.description
-	    && a->notes == b->notes;
+	    && a.notes == b.notes;
 }
 
-struct dive_site *get_same_dive_site(const struct dive_site *site)
+struct dive_site *get_same_dive_site(const struct dive_site &site)
 {
-	int i;
+	return get_dive_site_by_predicate(*divelog.sites,
-	struct dive_site *ds;
+			       [site](const auto &ds) { return same_dive_site(*ds, site); });
-	for_each_dive_site (i, ds, divelog.sites)
-		if (same_dive_site(ds, site))
-			return ds;
-	return NULL;
 }
 
 void merge_dive_site(struct dive_site *a, struct dive_site *b)
@@ -299,32 +226,27 @@ void merge_dive_site(struct dive_site *a, struct dive_site *b)
 		a->taxonomy = std::move(b->taxonomy);
 }
 
-struct dive_site *find_or_create_dive_site_with_name(const std::string &name, struct dive_site_table *ds_table)
+struct dive_site *find_or_create_dive_site_with_name(const std::string &name, dive_site_table &ds_table)
 {
-	int i;
+	struct dive_site *ds = get_dive_site_by_name(name, ds_table);
-	struct dive_site *ds;
-	for_each_dive_site(i,ds, ds_table) {
-		if (name == ds->name)
-			break;
-	}
 	if (ds)
 		return ds;
 	return create_dive_site(name, ds_table);
 }
 
-void purge_empty_dive_sites(struct dive_site_table *ds_table)
+void purge_empty_dive_sites(dive_site_table &ds_table)
 {
-	int i, j;
+	for (const auto &ds: ds_table) {
-	struct dive *d;
+		if (!dive_site_is_empty(ds.get()))
-	struct dive_site *ds;
-
-	for (i = 0; i < ds_table->nr; i++) {
-		ds = get_dive_site(i, ds_table);
-		if (!dive_site_is_empty(ds))
 			continue;
-		for_each_dive(j, d) {
+		while (!ds->dives.empty()) {
-			if (d->dive_site == ds)
+			struct dive *d = ds->dives.back();
+			if (d->dive_site != ds.get()) {
+				report_info("Warning: dive %d registered to wrong dive site in %s", d->number, __func__);
+				ds->dives.pop_back();
+			} else {
 				unregister_dive_from_dive_site(d);
+			}
 		}
 	}
 }

core/divesite.h

@@ -2,9 +2,10 @@
 #ifndef DIVESITE_H
 #define DIVESITE_H
 
-#include "units.h"
-#include "taxonomy.h"
 #include "divelist.h"
+#include "owning_table.h"
+#include "taxonomy.h"
+#include "units.h"
 #include <stdlib.h>
 
 #include <QObject>
@@ -21,52 +22,39 @@ struct dive_site
 	dive_site();
 	dive_site(const std::string &name);
 	dive_site(const std::string &name, const location_t *loc);
+	dive_site(uint32_t uuid);
 	~dive_site();
 };
 
-typedef struct dive_site_table {
+inline int divesite_comp_uuid(const dive_site &ds1, const dive_site &ds2)
-	int nr, allocated;
-	struct dive_site **dive_sites;
-} dive_site_table_t;
-
-static const dive_site_table_t empty_dive_site_table = { 0, 0, (struct dive_site **)0 };
-
-static inline struct dive_site *get_dive_site(int nr, struct dive_site_table *ds_table)
 {
-	if (nr >= ds_table->nr || nr < 0)
+	if (ds1.uuid == ds2.uuid)
-		return NULL;
+		return 0;
-	return ds_table->dive_sites[nr];
+	return ds1.uuid < ds2.uuid ? -1 : 1;
 }
 
-/* iterate over each dive site */
+class dive_site_table : public sorted_owning_table<dive_site, &divesite_comp_uuid> {
-#define for_each_dive_site(_i, _x, _ds_table) \
+public:
-	for ((_i) = 0; ((_x) = get_dive_site(_i, _ds_table)) != NULL; (_i)++)
+	put_result register_site(std::unique_ptr<dive_site> site);
+};
 
-int get_divesite_idx(const struct dive_site *ds, struct dive_site_table *ds_table);
+int get_divesite_idx(const struct dive_site *ds, dive_site_table &ds_table);
-struct dive_site *get_dive_site_by_uuid(uint32_t uuid, struct dive_site_table *ds_table);
+struct dive_site *get_dive_site_by_uuid(uint32_t uuid, dive_site_table &ds_table);
-void sort_dive_site_table(struct dive_site_table *ds_table);
+struct dive_site *alloc_or_get_dive_site(uint32_t uuid, dive_site_table &ds_table);
-int add_dive_site_to_table(struct dive_site *ds, struct dive_site_table *ds_table);
-struct dive_site *alloc_or_get_dive_site(uint32_t uuid, struct dive_site_table *ds_table);
-struct dive_site *alloc_dive_site();
 size_t nr_of_dives_at_dive_site(const struct dive_site &ds);
 bool is_dive_site_selected(const struct dive_site &ds);
-int unregister_dive_site(struct dive_site *ds);
+struct dive_site *create_dive_site(const std::string &name, dive_site_table &ds_table);
-int register_dive_site(struct dive_site *ds);
+struct dive_site *create_dive_site_with_gps(const std::string &name, const location_t *, dive_site_table &ds_table);
-void delete_dive_site(struct dive_site *ds, struct dive_site_table *ds_table);
+struct dive_site *get_dive_site_by_name(const std::string &name, dive_site_table &ds_table);
-struct dive_site *create_dive_site(const std::string &name, struct dive_site_table *ds_table);
+struct dive_site *get_dive_site_by_gps(const location_t *, dive_site_table &ds_table);
-struct dive_site *create_dive_site_with_gps(const std::string &name, const location_t *, struct dive_site_table *ds_table);
+struct dive_site *get_dive_site_by_gps_and_name(const std::string &name, const location_t *, dive_site_table &ds_table);
-struct dive_site *get_dive_site_by_name(const std::string &name, struct dive_site_table *ds_table);
+struct dive_site *get_dive_site_by_gps_proximity(const location_t *, int distance, dive_site_table &ds_table);
-struct dive_site *get_dive_site_by_gps(const location_t *, struct dive_site_table *ds_table);
+struct dive_site *get_same_dive_site(const struct dive_site &);
-struct dive_site *get_dive_site_by_gps_and_name(const std::string &name, const location_t *, struct dive_site_table *ds_table);
-struct dive_site *get_dive_site_by_gps_proximity(const location_t *, int distance, struct dive_site_table *ds_table);
-struct dive_site *get_same_dive_site(const struct dive_site *);
 bool dive_site_is_empty(struct dive_site *ds);
 void merge_dive_site(struct dive_site *a, struct dive_site *b);
 unsigned int get_distance(const location_t *loc1, const location_t *loc2);
-struct dive_site *find_or_create_dive_site_with_name(const std::string &name, struct dive_site_table *ds_table);
+struct dive_site *find_or_create_dive_site_with_name(const std::string &name, dive_site_table &ds_table);
-void purge_empty_dive_sites(struct dive_site_table *ds_table);
+void purge_empty_dive_sites(dive_site_table &ds_table);
-void clear_dive_site_table(struct dive_site_table *ds_table);
-void move_dive_site_table(struct dive_site_table *src, struct dive_site_table *dst);
 void add_dive_to_dive_site(struct dive *d, struct dive_site *ds);
 struct dive_site *unregister_dive_from_dive_site(struct dive *d);
 std::string constructLocationTags(const taxonomy_data &taxonomy, bool for_maintab);

core/import-cobalt.cpp

@@ -181,7 +181,7 @@ static int cobalt_dive(void *param, int, char **data, char **)
 
 	if (location && location_site) {
 		std::string tmp = std::string(location) + " / " + location_site;
-		add_dive_to_dive_site(state->cur_dive, find_or_create_dive_site_with_name(tmp, state->log->sites));
+		add_dive_to_dive_site(state->cur_dive, find_or_create_dive_site_with_name(tmp, *state->log->sites));
 	}
 	free(location);
 	free(location_site);

core/import-divinglog.cpp

@@ -276,7 +276,7 @@ static int divinglog_dive(void *param, int, char **data, char **)
 	state->cur_dive->when = (time_t)(atol(data[1]));
 
 	if (data[2])
-		add_dive_to_dive_site(state->cur_dive, find_or_create_dive_site_with_name(std::string(data[2]), state->log->sites));
+		add_dive_to_dive_site(state->cur_dive, find_or_create_dive_site_with_name(std::string(data[2]), *state->log->sites));
 
 	if (data[3])
 		utf8_string(data[3], &state->cur_dive->buddy);

core/libdivecomputer.cpp

@@ -636,7 +636,7 @@ static void parse_string_field(device_data_t *devdata, struct dive *dive, dc_fie
 
 		if (location.lat.udeg && location.lon.udeg) {
 			unregister_dive_from_dive_site(dive);
-			add_dive_to_dive_site(dive, create_dive_site_with_gps(std::string(str->value), &location, devdata->log->sites));
+			add_dive_to_dive_site(dive, create_dive_site_with_gps(std::string(str->value), &location, *devdata->log->sites));
 		}
 	}
 }

core/liquivision.cpp

@@ -131,7 +131,7 @@ static int handle_event_ver3(int code, const unsigned char *ps, unsigned int ps_
 	return skip;
 }
 
-static void parse_dives(int log_version, const unsigned char *buf, unsigned int buf_size, struct dive_table *table, struct dive_site_table *sites)
+static void parse_dives(int log_version, const unsigned char *buf, unsigned int buf_size, struct dive_table *table, dive_site_table &sites)
 {
 	unsigned int ptr = 0;
 	unsigned char model;
@@ -450,7 +450,7 @@ int try_to_open_liquivision(const char *, std::string &mem, struct divelog *log)
 	}
 	ptr += 4;
 
-	parse_dives(log_version, buf + ptr, buf_size - ptr, log->dives, log->sites);
+	parse_dives(log_version, buf + ptr, buf_size - ptr, log->dives, *log->sites);
 
 	return 1;
 }

core/load-git.cpp

@@ -175,9 +175,9 @@ static void parse_dive_gps(char *line, struct git_parser_state *state)
 
 	parse_location(line, &location);
 	if (!ds) {
-		ds = get_dive_site_by_gps(&location, state->log->sites);
+		ds = get_dive_site_by_gps(&location, *state->log->sites);
 		if (!ds)
-			ds = create_dive_site_with_gps(std::string(), &location, state->log->sites);
+			ds = create_dive_site_with_gps(std::string(), &location, *state->log->sites);
 		add_dive_to_dive_site(state->active_dive, ds);
 	} else {
 		if (dive_site_has_gps_location(ds) && ds->location != location) {
@@ -221,9 +221,9 @@ static void parse_dive_location(char *, struct git_parser_state *state)
 	std::string name = get_first_converted_string(state);
 	struct dive_site *ds = get_dive_site_for_dive(state->active_dive);
 	if (!ds) {
-		ds = get_dive_site_by_name(name, state->log->sites);
+		ds = get_dive_site_by_name(name, *state->log->sites);
 		if (!ds)
-			ds = create_dive_site(name, state->log->sites);
+			ds = create_dive_site(name, *state->log->sites);
 		add_dive_to_dive_site(state->active_dive, ds);
 	} else {
 		// we already had a dive site linked to the dive
@@ -252,7 +252,7 @@ static void parse_dive_notes(char *, struct git_parser_state *state)
 { state->active_dive->notes = get_first_converted_string_c(state); }
 
 static void parse_dive_divesiteid(char *line, struct git_parser_state *state)
-{ add_dive_to_dive_site(state->active_dive, get_dive_site_by_uuid(get_hex(line), state->log->sites)); }
+{ add_dive_to_dive_site(state->active_dive, get_dive_site_by_uuid(get_hex(line), *state->log->sites)); }
 
 /*
  * We can have multiple tags.
@@ -1711,7 +1711,7 @@ static int parse_site_entry(struct git_parser_state *state, const git_tree_entry
 	if (*suffix == '\0')
 		return report_error("Dive site without uuid");
 	uint32_t uuid = strtoul(suffix, NULL, 16);
-	state->active_site = alloc_or_get_dive_site(uuid, state->log->sites);
+	state->active_site = alloc_or_get_dive_site(uuid, *state->log->sites);
 	git_blob *blob = git_tree_entry_blob(state->repo, entry);
 	if (!blob)
 		return report_error("Unable to read dive site file");

core/owning_table.h

@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Traditionally, most of our data structures are collected as tables
+ * (=arrays) of pointers. The advantage is that pointers (or references)
+ * to objects are stable, even if the array grows and reallocates.
+ * Implicitly, the table is the owner of the objects and the objets
+ * are deleted (freed) when removed from the table.
+ * This header defines an std::vector<> of unique_ptr<>s to make this
+ * explicit.
+ *
+ * The state I want to reach across the code base: whenever a part of the
+ * code owns a heap allocated object, it *always* possesses a unique_ptr<>
+ * to that object. All naked pointers are invariably considered as
+ * "non owning".
+ *
+ * There are two ways to end ownership:
+ * 1) The std::unique_ptr<> goes out of scope and the object is
+ *    automatically deleted.
+ * 2) Ownership is passed to another std::unique_ptr<> using std::move().
+ *
+ * This means that when adding an object to an owning_table,
+ * ownership of a std::unique_ptr<> is given up with std::move().
+ * The table then returns a non-owning pointer to the object and
+ * optionally the insertion index.
+ *
+ * In converse, when removing an object, one provides a non-owning
+ * pointer, which is turned into an owning std::unique_ptr<> and
+ * the index where the object was removed.
+ * When ignoring the returned owning pointer, the object is
+ * automatically freed.
+ *
+ * The functions to add an entry to the table are called "put()",
+ * potentially with a suffix. The functions to remove an entry
+ * are called "pull()", likewise with an optional suffix.
+ *
+ * There are two versions of the table:
+ * 1) An unordered version, where the caller is responsible for
+ *    adding at specified positions (either given by an index or at the end).
+ *    Removal via a non-owning pointer is implemented by a linear search
+ *    over the whole table.
+ * 2) An ordered version, where a comparison function that returns -1, 0, 1
+ *    is used to add objects. In that case, the caller must make sure that
+ *    no two ojects that compare equal are added to the table.
+ *    Obviously, the compare function is supposed to be replaced by the
+ *    "spaceship operator" in due course.
+ *    Here, adding and removing via non-owning pointers is implemented
+ *    using a binary search.
+ *
+ * Note that, since the table contains std::unique_ptr<>s, to loop over
+ * all entries, it is best to use something such as
+ *      for (const auto &ptr: table) ...
+ * I plan to add iterator adapters, that autometically dereference
+ * the unique_ptr<>s and provide const-references for const-tables.
+ *
+ * Time will tell how useful this class is.
+ */
+#ifndef CORE_OWNING_TABLE_H
+#define CORE_OWNING_TABLE_H
+
+#include "errorhelper.h"
+
+#include <algorithm>
+#include <memory>
+#include <string>
+#include <vector>
+
+template <typename T>
+class owning_table : public std::vector<std::unique_ptr<T>> {
+public:
+	struct put_result {
+		T *ptr;
+		size_t idx;
+	};
+	struct pull_result {
+		std::unique_ptr<T> ptr;
+		size_t idx;
+	};
+	size_t get_idx(const T *item) const {
+		auto it = std::find_if(this->begin(), this->end(),
+				[item] (auto &i1) {return i1.get() == item; });
+		return it != this->end() ? it - this->begin() : std::string::npos;
+	}
+	T *put_at(std::unique_ptr<T> item, size_t idx) {
+		T *res = item.get();
+		insert(this->begin() + idx, std::move(item));
+		return res;
+	}
+	// Returns index of added item
+	put_result put_back(std::unique_ptr<T> item) {
+		T *ptr = item.get();
+		push_back(std::move(item));
+		return { ptr, this->size() - 1 };
+	}
+	std::unique_ptr<T> pull_at(size_t idx) {
+		auto it = this->begin() + idx;
+		std::unique_ptr<T> res = std::move(*it);
+		this->erase(it);
+		return res;
+	}
+	pull_result pull(const T *item) {
+		size_t idx = get_idx(item);
+		if (idx == std::string::npos) {
+			report_info("Warning: removing unexisting item in %s", __func__);
+			return { std::unique_ptr<T>(), std::string::npos };
+		}
+		return { pull_at(idx), idx };
+	}
+};
+
+// Note: there must not be any elements that compare equal!
+template <typename T, int (*CMP)(const T &, const T &)>
+class sorted_owning_table : public owning_table<T> {
+public:
+	using typename owning_table<T>::put_result;
+	using typename owning_table<T>::pull_result;
+	// Returns index of added item
+	put_result put(std::unique_ptr<T> item) {
+		auto it = std::lower_bound(this->begin(), this->end(), item,
+				[] (const auto &i1, const auto &i2)
+				{ return CMP(*i1, *i2) < 0; });
+		if (it != this->end() && CMP(**it, *item) == 0)
+			report_info("Warning: adding duplicate item in %s", __func__);
+		size_t idx = it - this->begin();
+		T *ptr = item.get();
+		this->insert(it, std::move(item));
+		return { ptr, idx };
+	}
+	// Optimized version of get_idx(), which uses binary search
+	// If not found, fall back to linear search and emit a warning.
+	// Note: this is probaly slower than a linesr search. But for now,
+	// it helps finding consistency problems.
+	size_t get_idx(const T *item) const {
+		auto it = std::lower_bound(this->begin(), this->end(), item,
+				[] (const auto &i1, const auto &i2)
+				{ return CMP(*i1, *i2) < 0; });
+		if (it == this->end() || CMP(**it, *item) != 0) {
+			size_t idx = owning_table<T>::get_idx(item);
+			if (idx != std::string::npos)
+				report_info("Warning: index found by linear but not by binary search in %s", __func__);
+			return idx;
+		}
+		return it - this->begin();
+	}
+	pull_result pull(const T *item) {
+		size_t idx = get_idx(item);
+		if (idx == std::string::npos) {
+			report_info("Warning: removing unexisting item in %s", __func__);
+			return { std::unique_ptr<T>(), std::string::npos };
+		}
+		return { this->pull_at(idx), idx };
+	}
+};
+
+#endif

core/parse-xml.cpp

@@ -559,7 +559,7 @@ static void dive_site(const char *buffer, struct dive *d, struct parser_state *s
 {
 	uint32_t uuid;
 	hex_value(buffer, &uuid);
-	add_dive_to_dive_site(d, get_dive_site_by_uuid(uuid, state->log->sites));
+	add_dive_to_dive_site(d, get_dive_site_by_uuid(uuid, *state->log->sites));
 }
 
 static void get_notrip(const char *buffer, bool *notrip)
@@ -977,9 +977,9 @@ static void divinglog_place(const char *place, struct dive *d, struct parser_sta
 		 !state->city.empty() ? state->city.c_str() : "",
 		 !state->country.empty() ? ", " : "",
 		 !state->country.empty() ? state->country.c_str() : "");
-	ds = get_dive_site_by_name(buffer, state->log->sites);
+	ds = get_dive_site_by_name(buffer, *state->log->sites);
 	if (!ds)
-		ds = create_dive_site(buffer, state->log->sites);
+		ds = create_dive_site(buffer, *state->log->sites);
 	add_dive_to_dive_site(d, ds);
 
 	// TODO: capture the country / city info in the taxonomy instead
@@ -1149,7 +1149,7 @@ static void gps_lat(const char *buffer, struct dive *dive, struct parser_state *
 
 	location.lat = parse_degrees(buffer, &end);
 	if (!ds) {
-		add_dive_to_dive_site(dive, create_dive_site_with_gps(std::string(), &location, state->log->sites));
+		add_dive_to_dive_site(dive, create_dive_site_with_gps(std::string(), &location, *state->log->sites));
 	} else {
 		if (ds->location.lat.udeg && ds->location.lat.udeg != location.lat.udeg)
 			report_info("Oops, changing the latitude of existing dive site id %8x name %s; not good", ds->uuid,
@@ -1166,7 +1166,7 @@ static void gps_long(const char *buffer, struct dive *dive, struct parser_state
 
 	location.lon = parse_degrees(buffer, &end);
 	if (!ds) {
-		add_dive_to_dive_site(dive, create_dive_site_with_gps(std::string(), &location, state->log->sites));
+		add_dive_to_dive_site(dive, create_dive_site_with_gps(std::string(), &location, *state->log->sites));
 	} else {
 		if (ds->location.lon.udeg && ds->location.lon.udeg != location.lon.udeg)
 			report_info("Oops, changing the longitude of existing dive site id %8x name %s; not good", ds->uuid,
@@ -1197,14 +1197,14 @@ static void gps_in_dive(const char *buffer, struct dive *dive, struct parser_sta
 	parse_location(buffer, &location);
 	if (!ds) {
 		// check if we have a dive site within 20 meters of that gps fix
-		ds = get_dive_site_by_gps_proximity(&location, 20, state->log->sites);
+		ds = get_dive_site_by_gps_proximity(&location, 20, *state->log->sites);
 
 		if (ds) {
 			// found a site nearby; in case it turns out this one had a different name let's
 			// remember the original coordinates so we can create the correct dive site later
 			state->cur_location = location;
 		} else {
-			ds = create_dive_site_with_gps(std::string(), &location, state->log->sites);
+			ds = create_dive_site_with_gps(std::string(), &location, *state->log->sites);
 		}
 		add_dive_to_dive_site(dive, ds);
 	} else {
@@ -2225,7 +2225,7 @@ int parse_dlf_buffer(unsigned char *buffer, size_t size, struct divelog *log)
 				/* Measure GPS */
 				state.cur_location.lat.udeg =  (int)((ptr[7]  << 24) + (ptr[6]  << 16) + (ptr[5] << 8) + (ptr[4] << 0));
 				state.cur_location.lon.udeg = (int)((ptr[11] << 24) + (ptr[10] << 16) + (ptr[9] << 8) + (ptr[8] << 0));
-				add_dive_to_dive_site(state.cur_dive, create_dive_site_with_gps("DLF imported"s, &state.cur_location, state.log->sites));
+				add_dive_to_dive_site(state.cur_dive, create_dive_site_with_gps("DLF imported"s, &state.cur_location, *state.log->sites));
 				break;
 			default:
 				break;

core/parse.cpp

@@ -199,7 +199,7 @@ void dive_site_end(struct parser_state *state)
 	if (!state->cur_dive_site)
 		return;
 
-	struct dive_site *ds = alloc_or_get_dive_site(state->cur_dive_site->uuid, state->log->sites);
+	struct dive_site *ds = alloc_or_get_dive_site(state->cur_dive_site->uuid, *state->log->sites);
 	merge_dive_site(ds, state->cur_dive_site.get());
 
 	if (verbose > 3)
@@ -470,7 +470,7 @@ void add_dive_site(const char *ds_name, struct dive *dive, struct parser_state *
 		struct dive_site *ds = dive->dive_site;
 		if (!ds) {
 			// if the dive doesn't have a dive site, check if there's already a dive site by this name
-			ds = get_dive_site_by_name(trimmed, state->log->sites);
+			ds = get_dive_site_by_name(trimmed, *state->log->sites);
 		}
 		if (ds) {
 			// we have a dive site, let's hope there isn't a different name
@@ -481,12 +481,12 @@ void add_dive_site(const char *ds_name, struct dive *dive, struct parser_state *
 				// but wait, we could have gotten this one based on GPS coords and could
 				// have had two different names for the same site... so let's search the other
 				// way around
-				struct dive_site *exact_match = get_dive_site_by_gps_and_name(trimmed, &ds->location, state->log->sites);
+				struct dive_site *exact_match = get_dive_site_by_gps_and_name(trimmed, &ds->location, *state->log->sites);
 				if (exact_match) {
 					unregister_dive_from_dive_site(dive);
 					add_dive_to_dive_site(dive, exact_match);
 				} else {
-					struct dive_site *newds = create_dive_site(trimmed.c_str(), state->log->sites);
+					struct dive_site *newds = create_dive_site(trimmed.c_str(), *state->log->sites);
 					unregister_dive_from_dive_site(dive);
 					add_dive_to_dive_site(dive, newds);
 					if (has_location(&state->cur_location)) {
@@ -504,7 +504,7 @@ void add_dive_site(const char *ds_name, struct dive *dive, struct parser_state *
 				add_dive_to_dive_site(dive, ds);
 			}
 		} else {
-			add_dive_to_dive_site(dive, create_dive_site(trimmed, state->log->sites));
+			add_dive_to_dive_site(dive, create_dive_site(trimmed, *state->log->sites));
 		}
 	}
 }

core/save-git.cpp

@@ -922,10 +922,9 @@ static void save_divesites(git_repository *repo, struct dir *tree)
 	put_format(&dirname, "01-Divesites");
 	subdir = new_directory(repo, tree, &dirname);
 
-	purge_empty_dive_sites(divelog.sites);
+	purge_empty_dive_sites(*divelog.sites);
-	for (int i = 0; i < divelog.sites->nr; i++) {
+	for (const auto &ds: *divelog.sites) {
 		membuffer b;
-		struct dive_site *ds = get_dive_site(i, divelog.sites);
 		membuffer site_file_name;
 		put_format(&site_file_name, "Site-%08x", ds->uuid);
 		show_utf8(&b, "name ", ds->name.c_str(), "\n");

core/save-xml.cpp

@@ -701,10 +701,9 @@ static void save_dives_buffer(struct membuffer *b, bool select_only, bool anonym
 
 	/* save the dive sites */
 	put_format(b, "<divesites>\n");
-	for (i = 0; i < divelog.sites->nr; i++) {
+	for (const auto &ds: *divelog.sites) {
-		struct dive_site *ds = get_dive_site(i, divelog.sites);
 		/* Don't export empty dive sites */
-		if (dive_site_is_empty(ds))
+		if (dive_site_is_empty(ds.get()))
 			continue;
 		/* Only write used dive sites when exporting selected dives */
 		if (select_only && !is_dive_site_selected(*ds))

core/uemis-downloader.cpp

@@ -915,7 +915,7 @@ static bool process_raw_buffer(device_data_t *devdata, uint32_t deviceid, std::s
 		} else if (!is_log && dive && tag == "divespot_id") {
 			int divespot_id;
 			if (from_chars(val, divespot_id).ec != std::errc::invalid_argument) {
-				struct dive_site *ds = create_dive_site("from Uemis"s, devdata->log->sites);
+				struct dive_site *ds = create_dive_site("from Uemis"s, *devdata->log->sites);
 				unregister_dive_from_dive_site(dive);
 				add_dive_to_dive_site(dive, ds);
 				uemis_obj.mark_divelocation(dive->dc.diveid, divespot_id, ds);
@@ -1109,21 +1109,20 @@ static void get_uemis_divespot(device_data_t *devdata, const std::string &mountp
 			 * we search all existing divesites if we have one with the same name already. The function
 			 * returns the first found which is luckily not the newly created.
 			 */
-			struct dive_site *ods = get_dive_site_by_name(nds->name, devdata->log->sites);
+			struct dive_site *ods = get_dive_site_by_name(nds->name, *devdata->log->sites);
-			if (ods) {
+			if (ods && nds->uuid != ods->uuid) {
 				/* if the uuid's are the same, the new site is a duplicate and can be deleted */
-				if (nds->uuid != ods->uuid) {
+				unregister_dive_from_dive_site(dive);
-					delete_dive_site(nds, devdata->log->sites);
+				add_dive_to_dive_site(dive, ods);
-					unregister_dive_from_dive_site(dive);
+				devdata->log->sites->pull(nds);
-					add_dive_to_dive_site(dive, ods);
-				}
 			}
 			divespot_mapping[divespot_id] = dive->dive_site;
 		} else {
 			/* if we can't load the dive site details, delete the site we
 			* created in process_raw_buffer
 			*/
-			delete_dive_site(dive->dive_site, devdata->log->sites);
+			devdata->log->sites->pull(dive->dive_site);
+			dive->dive_site = nullptr;
 		}
 	}
 }

desktop-widgets/divesiteimportdialog.cpp

@@ -10,20 +10,18 @@
 
 #include <QShortcut>
 
-// Caller keeps ownership of "imported". The contents of "imported" will be consumed on execution of the dialog.
+DivesiteImportDialog::DivesiteImportDialog(dive_site_table imported, QString source, QWidget *parent) : QDialog(parent),
-// On return, it will be empty.
+	importedSites(std::move(imported)),
-DivesiteImportDialog::DivesiteImportDialog(struct dive_site_table &imported, QString source, QWidget *parent) : QDialog(parent),
+	importedSource(std::move(source)),
-	importedSource(std::move(source))
+	divesiteImportedModel(std::make_unique<DivesiteImportedModel>())
 {
 	QShortcut *close = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_W), this);
 	QShortcut *quit = new QShortcut(QKeySequence(Qt::CTRL | Qt::Key_Q), this);
 
-	divesiteImportedModel = new DivesiteImportedModel(this);
-
 	int startingWidth = defaultModelFont().pointSize();
 
 	ui.setupUi(this);
-	ui.importedDivesitesView->setModel(divesiteImportedModel);
+	ui.importedDivesitesView->setModel(divesiteImportedModel.get());
 	ui.importedDivesitesView->setSelectionBehavior(QAbstractItemView::SelectRows);
 	ui.importedDivesitesView->setSelectionMode(QAbstractItemView::SingleSelection);
 	ui.importedDivesitesView->horizontalHeader()->setStretchLastSection(true);
@@ -35,45 +33,36 @@ DivesiteImportDialog::DivesiteImportDialog(struct dive_site_table &imported, QSt
 	ui.selectAllButton->setEnabled(true);
 	ui.unselectAllButton->setEnabled(true);
 
-	connect(ui.importedDivesitesView, &QTableView::clicked, divesiteImportedModel, &DivesiteImportedModel::changeSelected);
+	connect(ui.importedDivesitesView, &QTableView::clicked, divesiteImportedModel.get(), &DivesiteImportedModel::changeSelected);
-	connect(ui.selectAllButton, &QPushButton::clicked, divesiteImportedModel, &DivesiteImportedModel::selectAll);
+	connect(ui.selectAllButton, &QPushButton::clicked, divesiteImportedModel.get(), &DivesiteImportedModel::selectAll);
-	connect(ui.unselectAllButton, &QPushButton::clicked, divesiteImportedModel, &DivesiteImportedModel::selectNone);
+	connect(ui.unselectAllButton, &QPushButton::clicked, divesiteImportedModel.get(), &DivesiteImportedModel::selectNone);
 	connect(close, SIGNAL(activated()), this, SLOT(close()));
 	connect(quit, SIGNAL(activated()), parent, SLOT(close()));
 
 	ui.ok->setEnabled(true);
 
-	importedSites = imported;
-	imported.nr = imported.allocated = 0;
-	imported.dive_sites = nullptr;
-
 	divesiteImportedModel->repopulate(&importedSites);
 }
 
 DivesiteImportDialog::~DivesiteImportDialog()
 {
-	clear_dive_site_table(&importedSites);
 }
 
 void DivesiteImportDialog::on_cancel_clicked()
 {
-	clear_dive_site_table(&importedSites);
 	done(-1);
 }
 
 void DivesiteImportDialog::on_ok_clicked()
 {
 	// delete non-selected dive sites
-	struct dive_site_table selectedSites = empty_dive_site_table;
+	dive_site_table selectedSites;
-	for (int i = 0; i < importedSites.nr; i++)
+	for (size_t i = 0; i < importedSites.size(); i++)  {
-		if (divesiteImportedModel->data(divesiteImportedModel->index(i, 0), Qt::CheckStateRole) == Qt::Checked) {
+		if (divesiteImportedModel->data(divesiteImportedModel->index(i, 0), Qt::CheckStateRole) == Qt::Checked)
-			struct dive_site *newSite = new dive_site;
+			selectedSites.push_back(std::move(importedSites[i]));
-			*newSite = *importedSites.dive_sites[i];
+	}
-			add_dive_site_to_table(newSite, &selectedSites);
+	importedSites.clear(); // Hopefully, the model is not used thereafter!
-		}
 
-	Command::importDiveSites(&selectedSites, importedSource);
+	Command::importDiveSites(std::move(selectedSites), importedSource);
-	clear_dive_site_table(&selectedSites);
-	clear_dive_site_table(&importedSites);
 	accept();
 }

desktop-widgets/divesiteimportdialog.h

@@ -21,7 +21,8 @@ class DivesiteImportedModel;
 class DivesiteImportDialog : public QDialog {
 	Q_OBJECT
 public:
-	DivesiteImportDialog(struct dive_site_table &imported, QString source, QWidget *parent = 0 );
+	// Note: takes ownership of importedd table
+	DivesiteImportDialog(dive_site_table imported, QString source, QWidget *parent = 0);
 	~DivesiteImportDialog();
 
 public
@@ -31,10 +32,10 @@ slots:
 
 private:
 	Ui::DivesiteImportDialog ui;
-	struct dive_site_table importedSites;
+	dive_site_table importedSites;
 	QString importedSource;
 
-	DivesiteImportedModel *divesiteImportedModel;
+	std::unique_ptr<DivesiteImportedModel> divesiteImportedModel;
 };
 
 #endif // DIVESITEIMPORTDIALOG_H

desktop-widgets/divesitelistview.cpp

@@ -97,7 +97,7 @@ void DiveSiteListView::diveSiteAdded(struct dive_site *, int idx)
 
 void DiveSiteListView::diveSiteChanged(struct dive_site *ds, int field)
 {
-	int idx = get_divesite_idx(ds, divelog.sites);
+	int idx = get_divesite_idx(ds, *divelog.sites);
 	if (idx < 0)
 		return;
 	QModelIndex globalIdx = LocationInformationModel::instance()->index(idx, field);

desktop-widgets/locationinformation.cpp

@@ -388,9 +388,9 @@ bool DiveLocationFilterProxyModel::lessThan(const QModelIndex &source_left, cons
 	// If there is a current location, sort by that - otherwise use the provided column
 	if (has_location(&currentLocation)) {
 		// The dive sites are -2 because of the first two items.
-		struct dive_site *ds1 = get_dive_site(source_left.row() - 2, divelog.sites);
+		auto loc1 = (*divelog.sites)[source_left.row() - 2]->location;
-		struct dive_site *ds2 = get_dive_site(source_right.row() - 2, divelog.sites);
+		auto loc2 = (*divelog.sites)[source_right.row() - 2]->location;
-		return get_distance(&ds1->location, &currentLocation) < get_distance(&ds2->location, &currentLocation);
+		return get_distance(&loc1, &currentLocation) < get_distance(&loc2, &currentLocation);
 	}
 	return source_left.data().toString().compare(source_right.data().toString(), Qt::CaseInsensitive) < 0;
 }
@@ -411,6 +411,9 @@ QVariant DiveLocationModel::data(const QModelIndex &index, int role) const
 	static const QIcon plusIcon(":list-add-icon");
 	static const QIcon geoCode(":geotag-icon");
 
+	if (index.row() < 0 || index.row() >= (int)divelog.sites->size() + 2)
+		return QVariant();
+
 	if (index.row() <= 1) { // two special cases.
 		if (index.column() == LocationInformationModel::DIVESITE)
 			return QVariant::fromValue<dive_site *>(RECENTLY_ADDED_DIVESITE);
@@ -428,8 +431,8 @@ QVariant DiveLocationModel::data(const QModelIndex &index, int role) const
 	}
 
 	// The dive sites are -2 because of the first two items.
-	struct dive_site *ds = get_dive_site(index.row() - 2, divelog.sites);
+	const auto &ds = (*divelog.sites)[index.row() - 2];
-	return LocationInformationModel::getDiveSiteData(ds, index.column(), role);
+	return LocationInformationModel::getDiveSiteData(*ds, index.column(), role);
 }
 
 int DiveLocationModel::columnCount(const QModelIndex&) const
@@ -439,7 +442,7 @@ int DiveLocationModel::columnCount(const QModelIndex&) const
 
 int DiveLocationModel::rowCount(const QModelIndex&) const
 {
-	return divelog.sites->nr + 2;
+	return (int)divelog.sites->size() + 2;
 }
 
 Qt::ItemFlags DiveLocationModel::flags(const QModelIndex &index) const
@@ -560,12 +563,10 @@ void DiveLocationLineEdit::refreshDiveSiteCache()
 
 static struct dive_site *get_dive_site_name_start_which_str(const QString &str)
 {
-	struct dive_site *ds;
+	for (const auto &ds: *divelog.sites) {
-	int i;
-	for_each_dive_site (i, ds, divelog.sites) {
 		QString dsName = QString::fromStdString(ds->name);
 		if (dsName.toLower().startsWith(str.toLower()))
-			return ds;
+			return ds.get();
 	}
 	return NULL;
 }

desktop-widgets/mainwindow.cpp

@@ -1409,12 +1409,12 @@ void MainWindow::on_actionImportDiveSites_triggered()
 		parse_file(fileNamePtr.data(), &log);
 	}
 	// The imported dive sites still have pointers to imported dives - remove them
-	for (int i = 0; i < log.sites->nr; ++i)
+	for (const auto &ds: *log.sites)
-		log.sites->dive_sites[i]->dives.clear();
+		ds->dives.clear();
 
 	QString source = fileNames.size() == 1 ? fileNames[0] : tr("multiple files");
 
-	DivesiteImportDialog divesiteImport(*log.sites, source, this);
+	DivesiteImportDialog divesiteImport(std::move(*log.sites), source, this);
 	divesiteImport.exec();
 }
 

mobile-widgets/qmlmanager.cpp

@@ -1069,7 +1069,7 @@ bool QMLManager::checkLocation(DiveSiteChange &res, struct dive *d, QString loca
 	bool changed = false;
 	QString oldLocation = QString::fromStdString(get_dive_location(d));
 	if (oldLocation != location) {
-		ds = get_dive_site_by_name(location.toStdString(), divelog.sites);
+		ds = get_dive_site_by_name(location.toStdString(), *divelog.sites);
 		if (!ds && !location.isEmpty()) {
 			res.createdDs = std::make_unique<dive_site>(qPrintable(location));
 			res.changed = true;
@@ -1816,7 +1816,7 @@ QString QMLManager::getVersion() const
 
 QString QMLManager::getGpsFromSiteName(const QString &siteName)
 {
-	struct dive_site *ds = get_dive_site_by_name(siteName.toStdString(), divelog.sites);
+	struct dive_site *ds = get_dive_site_by_name(siteName.toStdString(), *divelog.sites);
 	if (!ds)
 		return QString();
 	return printGPSCoords(&ds->location);

qt-models/divelocationmodel.cpp

@@ -37,7 +37,7 @@ int LocationInformationModel::columnCount(const QModelIndex &) const
 
 int LocationInformationModel::rowCount(const QModelIndex &) const
 {
-	return divelog.sites->nr;
+	return (int)divelog.sites->size();
 }
 
 QVariant LocationInformationModel::headerData(int section, Qt::Orientation orientation, int role) const
@@ -81,21 +81,18 @@ Qt::ItemFlags LocationInformationModel::flags(const QModelIndex &index) const
 	return QAbstractItemModel::flags(index);
 }
 
-QVariant LocationInformationModel::getDiveSiteData(const struct dive_site *ds, int column, int role)
+QVariant LocationInformationModel::getDiveSiteData(const struct dive_site &ds, int column, int role)
 {
-	if (!ds)
-		return QVariant();
-
 	switch(role) {
 	case Qt::EditRole:
 	case Qt::DisplayRole:
 		switch(column) {
-		case DIVESITE: return QVariant::fromValue<dive_site *>((dive_site *)ds); // Not nice: casting away const
+		case DIVESITE: return QVariant::fromValue<dive_site *>((dive_site *)&ds); // Not nice: casting away const
-		case NAME: return QString::fromStdString(ds->name);
+		case NAME: return QString::fromStdString(ds.name);
-		case NUM_DIVES: return static_cast<int>(ds->dives.size());
+		case NUM_DIVES: return static_cast<int>(ds.dives.size());
 		case LOCATION: return "TODO";
-		case DESCRIPTION: return QString::fromStdString(ds->description);
+		case DESCRIPTION: return QString::fromStdString(ds.description);
-		case NOTES: return QString::fromStdString(ds->notes);
+		case NOTES: return QString::fromStdString(ds.notes);
 		case TAXONOMY: return "TODO";
 		}
 	break;
@@ -111,22 +108,22 @@ QVariant LocationInformationModel::getDiveSiteData(const struct dive_site *ds, i
 		case EDIT: return editIcon();
 		case REMOVE: return trashIcon();
 #endif
-		case NAME: return dive_site_has_gps_location(ds) ? QIcon(":geotag-icon") : QVariant();
+		case NAME: return dive_site_has_gps_location(&ds) ? QIcon(":geotag-icon") : QVariant();
 		}
 	break;
 	case DIVESITE_ROLE:
-		return QVariant::fromValue<dive_site *>((dive_site *)ds); // Not nice: casting away const
+		return QVariant::fromValue<dive_site *>((dive_site *)&ds); // Not nice: casting away const
 	}
 	return QVariant();
 }
 
 QVariant LocationInformationModel::data(const QModelIndex &index, int role) const
 {
-	if (!index.isValid())
+	if (!index.isValid() || index.row() >= (int)divelog.sites->size())
 		return QVariant();
 
-	struct dive_site *ds = get_dive_site(index.row(), divelog.sites);
+	const auto &ds = (*divelog.sites)[index.row()].get();
-	return getDiveSiteData(ds, index.column(), role);
+	return getDiveSiteData(*ds, index.column(), role);
 }
 
 void LocationInformationModel::update()
@@ -137,7 +134,7 @@ void LocationInformationModel::update()
 
 void LocationInformationModel::diveSiteDiveCountChanged(dive_site *ds)
 {
-	int idx = get_divesite_idx(ds, divelog.sites);
+	int idx = get_divesite_idx(ds, *divelog.sites);
 	if (idx >= 0)
 		dataChanged(createIndex(idx, NUM_DIVES), createIndex(idx, NUM_DIVES));
 }
@@ -162,7 +159,7 @@ void LocationInformationModel::diveSiteDeleted(struct dive_site *, int idx)
 
 void LocationInformationModel::diveSiteChanged(struct dive_site *ds, int field)
 {
-	int idx = get_divesite_idx(ds, divelog.sites);
+	int idx = get_divesite_idx(ds, *divelog.sites);
 	if (idx < 0)
 		return;
 	dataChanged(createIndex(idx, field), createIndex(idx, field));
@@ -170,7 +167,7 @@ void LocationInformationModel::diveSiteChanged(struct dive_site *ds, int field)
 
 void LocationInformationModel::diveSiteDivesChanged(struct dive_site *ds)
 {
-	int idx = get_divesite_idx(ds, divelog.sites);
+	int idx = get_divesite_idx(ds, *divelog.sites);
 	if (idx < 0)
 		return;
 	dataChanged(createIndex(idx, NUM_DIVES), createIndex(idx, NUM_DIVES));
@@ -181,9 +178,9 @@ bool DiveSiteSortedModel::filterAcceptsRow(int sourceRow, const QModelIndex &sou
 	if (fullText.isEmpty())
 		return true;
 
-	if (sourceRow < 0 || sourceRow > divelog.sites->nr)
+	if (sourceRow < 0 || sourceRow > (int)divelog.sites->size())
 		return false;
-	struct dive_site *ds = divelog.sites->dive_sites[sourceRow];
+	const auto &ds = (*divelog.sites)[sourceRow];
 	QString text = QString::fromStdString(ds->name + ds->description + ds->notes);
 	return text.contains(fullText, Qt::CaseInsensitive);
 }
@@ -193,10 +190,15 @@ bool DiveSiteSortedModel::lessThan(const QModelIndex &i1, const QModelIndex &i2)
 	// The source indices correspond to indices in the global dive site table.
 	// Let's access them directly without going via the source model.
 	// Kind of dirty, but less effort.
-	struct dive_site *ds1 = get_dive_site(i1.row(), divelog.sites);
+
-	struct dive_site *ds2 = get_dive_site(i2.row(), divelog.sites);
+	// Be careful to respect proper ordering when sites are invalid.
-	if (!ds1 || !ds2) // Invalid dive sites compare as different
+	bool valid1 = i1.row() >= 0 && i1.row() < (int)divelog.sites->size();
-		return false;
+	bool valid2 = i2.row() >= 0 && i2.row() < (int)divelog.sites->size();
+	if (!valid1 || !valid2)
+		return valid1 < valid2;
+
+	const auto &ds1 = (*divelog.sites)[i1.row()];
+	const auto &ds2 = (*divelog.sites)[i2.row()];
 	switch (i1.column()) {
 	case LocationInformationModel::NAME:
 	default:
@@ -230,18 +232,19 @@ QStringList DiveSiteSortedModel::allSiteNames() const
 		// This shouldn't happen, but if model and core get out of sync,
 		// (more precisely: the core has more sites than the model is aware of),
 		// we might get an invalid index.
-		if (idx < 0 || idx > divelog.sites->nr) {
+		if (idx < 0 || idx > (int)divelog.sites->size()) {
 			report_info("DiveSiteSortedModel::allSiteNames(): invalid index");
 			continue;
 		}
-		locationNames << QString::fromStdString(divelog.sites->dive_sites[idx]->name);
+		locationNames << QString::fromStdString((*divelog.sites)[idx]->name);
 	}
 	return locationNames;
 }
 
-struct dive_site *DiveSiteSortedModel::getDiveSite(const QModelIndex &idx)
+struct dive_site *DiveSiteSortedModel::getDiveSite(const QModelIndex &idx_source)
 {
-	return get_dive_site(mapToSource(idx).row(), divelog.sites);
+	auto idx = mapToSource(idx_source).row();
+	return idx >= 0 && idx < (int)divelog.sites->size() ? (*divelog.sites)[idx].get() : NULL;
 }
 
 #ifndef SUBSURFACE_MOBILE

qt-models/divelocationmodel.h

@@ -19,7 +19,7 @@ public:
 	// Thus, different views can connect to different models.
 	enum Columns { EDIT, REMOVE, NAME, DESCRIPTION, NUM_DIVES, LOCATION, NOTES, DIVESITE, TAXONOMY, COLUMNS };
 	enum Roles { DIVESITE_ROLE = Qt::UserRole + 1 };
-	static QVariant getDiveSiteData(const struct dive_site *ds, int column, int role);
+	static QVariant getDiveSiteData(const struct dive_site &ds, int column, int role);
 
 	LocationInformationModel(QObject *obj = 0);
 	static LocationInformationModel *instance();

qt-models/divesiteimportmodel.cpp

@@ -50,9 +50,9 @@ QVariant DivesiteImportedModel::data(const QModelIndex &index, int role) const
 	if (index.row() + firstIndex > lastIndex)
 		return QVariant();
 
-	struct dive_site *ds = get_dive_site(index.row() + firstIndex, importedSitesTable);
+	if (index.row() < 0 || index.row() >= (int)importedSitesTable->size())
-	if (!ds)
 		return QVariant();
+	struct dive_site *ds = (*importedSitesTable)[index.row()].get();
 
 	// widgets access the model via index.column()
 	// Not supporting QML access via roles
@@ -68,7 +68,7 @@ QVariant DivesiteImportedModel::data(const QModelIndex &index, int role) const
 			// 40075000 is circumference of the earth in meters
 			struct dive_site *nearest_ds =
 				get_dive_site_by_gps_proximity(&ds->location,
-				40075000, divelog.sites);
+				40075000, *divelog.sites);
 			if (nearest_ds)
 				return QString::fromStdString(nearest_ds->name);
 			else
@@ -78,7 +78,7 @@ QVariant DivesiteImportedModel::data(const QModelIndex &index, int role) const
 			unsigned int distance = 0;
 			struct dive_site *nearest_ds =
 				get_dive_site_by_gps_proximity(&ds->location,
-				40075000, divelog.sites);
+				40075000, *divelog.sites);
 			if (nearest_ds)
 				distance = get_distance(&ds->location,
 					&nearest_ds->location);
@@ -126,18 +126,15 @@ Qt::ItemFlags DivesiteImportedModel::flags(const QModelIndex &index) const
 	return QAbstractTableModel::flags(index) | Qt::ItemIsUserCheckable;
 }
 
-void DivesiteImportedModel::repopulate(struct dive_site_table *sites)
+void DivesiteImportedModel::repopulate(dive_site_table *sites)
 {
 	beginResetModel();
 
 	importedSitesTable = sites;
 	firstIndex = 0;
-	lastIndex = importedSitesTable->nr - 1;
+	lastIndex = (int)importedSitesTable->size() - 1; // Qt: the "last index" is negative for empty lists. Insane.
-	checkStates.resize(importedSitesTable->nr);
+	checkStates.resize(importedSitesTable->size());
-	for (int row = 0; row < importedSitesTable->nr; row++)
+	for (size_t row = 0; row < importedSitesTable->size(); row++)
-		if (get_dive_site_by_gps(&importedSitesTable->dive_sites[row]->location, divelog.sites))
+		checkStates[row] = !get_dive_site_by_gps(&(*importedSitesTable)[row]->location, *divelog.sites);
-			checkStates[row] = false;
-		else
-			checkStates[row] = true;
 	endResetModel();
 }

qt-models/divesiteimportmodel.h

@@ -17,7 +17,7 @@ public:
 	QVariant data(const QModelIndex& index, int role) const;
 	QVariant headerData(int section, Qt::Orientation orientation, int role) const;
 	Qt::ItemFlags flags(const QModelIndex &index) const;
-	void repopulate(dive_site_table_t *sites);
+	void repopulate(dive_site_table *sites);
 public
 slots:
 	void changeSelected(QModelIndex clickedIndex);
@@ -29,7 +29,7 @@ private:
 	int firstIndex;
 	int lastIndex;
 	std::vector<char> checkStates; // char instead of bool to avoid silly pessimization of std::vector.
-	struct dive_site_table *importedSitesTable;
+	dive_site_table *importedSitesTable;
 };
 
 #endif

qt-models/maplocationmodel.cpp

@@ -118,15 +118,15 @@ const QVector<dive_site *> &MapLocationModel::selectedDs() const
 	return m_selectedDs;
 }
 
-static bool hasVisibleDive(const dive_site *ds)
+static bool hasVisibleDive(const dive_site &ds)
 {
-	return std::any_of(ds->dives.begin(), ds->dives.end(),
+	return std::any_of(ds.dives.begin(), ds.dives.end(),
 			   [] (const dive *d) { return !d->hidden_by_filter; });
 }
 
-static bool hasSelectedDive(const dive_site *ds)
+static bool hasSelectedDive(const dive_site &ds)
 {
-	return std::any_of(ds->dives.begin(), ds->dives.end(),
+	return std::any_of(ds.dives.begin(), ds.dives.end(),
 			   [] (const dive *d) { return d->selected; });
 }
 
@@ -160,18 +160,17 @@ void MapLocationModel::reload(QObject *map)
 	if (diveSiteMode)
 		m_selectedDs = DiveFilter::instance()->filteredDiveSites();
 #endif
-	for (int i = 0; i < divelog.sites->nr; ++i) {
+	for (const auto &ds: *divelog.sites) {
-		struct dive_site *ds = divelog.sites->dive_sites[i];
 		QGeoCoordinate dsCoord;
 
 		// Don't show dive sites of hidden dives, unless we're in dive site edit mode.
-		if (!diveSiteMode && !hasVisibleDive(ds))
+		if (!diveSiteMode && !hasVisibleDive(*ds))
 			continue;
-		if (!dive_site_has_gps_location(ds)) {
+		if (!dive_site_has_gps_location(ds.get())) {
 			// Dive sites that do not have a gps location are not shown in normal mode.
 			// In dive-edit mode, selected sites are placed at the center of the map,
 			// so that the user can drag them somewhere without having to enter coordinates.
-			if (!diveSiteMode || !m_selectedDs.contains(ds) || !map)
+			if (!diveSiteMode || !m_selectedDs.contains(ds.get()) || !map)
 				continue;
 			dsCoord = map->property("center").value<QGeoCoordinate>();
 		} else {
@@ -179,8 +178,8 @@ void MapLocationModel::reload(QObject *map)
 			qreal longitude = ds->location.lon.udeg * 0.000001;
 			dsCoord = QGeoCoordinate(latitude, longitude);
 		}
-		if (!diveSiteMode && hasSelectedDive(ds) && !m_selectedDs.contains(ds))
+		if (!diveSiteMode && hasSelectedDive(*ds) && !m_selectedDs.contains(ds.get()))
-			m_selectedDs.append(ds);
+			m_selectedDs.append(ds.get());
 		QString name = siteMapDisplayName(ds->name);
 		if (!diveSiteMode) {
 			// don't add dive locations with the same name, unless they are
@@ -192,8 +191,8 @@ void MapLocationModel::reload(QObject *map)
 					continue;
 			}
 		}
-		bool selected = m_selectedDs.contains(ds);
+		bool selected = m_selectedDs.contains(ds.get());
-		MapLocation *location = new MapLocation(ds, dsCoord, name, selected);
+		MapLocation *location = new MapLocation(ds.get(), dsCoord, name, selected);
 		m_mapLocations.append(location);
 		if (!diveSiteMode)
 			locationNameMap[name] = location;

smtk-import/smartrak.cpp

@@ -423,12 +423,12 @@ static void smtk_build_location(MdbHandle *mdb, char *idx, struct dive_site **lo
 		concat(str, ", ", table.get_string_view(1)); // Locality
 	concat(str, ", ", site);
 
-	ds = get_dive_site_by_name(str, log->sites);
+	ds = get_dive_site_by_name(str, *log->sites);
 	if (!ds) {
 		if (!has_location(&loc))
-			ds = create_dive_site(str, log->sites);
+			ds = create_dive_site(str, *log->sites);
 		else
-			ds = create_dive_site_with_gps(str, &loc, log->sites);
+			ds = create_dive_site_with_gps(str, &loc, *log->sites);
 	}
 	*location = ds;
 

tests/testdivesiteduplication.cpp

@@ -9,7 +9,7 @@ void TestDiveSiteDuplication::testReadV2()
 {
 	prefs.cloud_base_url = strdup(default_prefs.cloud_base_url);
 	QCOMPARE(parse_file(SUBSURFACE_TEST_DATA "/dives/TwoTimesTwo.ssrf", &divelog), 0);
-	QCOMPARE(divelog.sites->nr, 2);
+	QCOMPARE(divelog.sites->size(), 2);
 }
 
 QTEST_GUILESS_MAIN(TestDiveSiteDuplication)

tests/testparse.cpp

@@ -92,7 +92,7 @@ int TestParse::parseCSV(int units, std::string file)
 int TestParse::parseDivingLog()
 {
 	// Parsing of DivingLog import from SQLite database
-	struct dive_site *ds = alloc_or_get_dive_site(0xdeadbeef, divelog.sites);
+	struct dive_site *ds = alloc_or_get_dive_site(0xdeadbeef, *divelog.sites);
 	ds->name = "Suomi -  - Hälvälä";
 
 	int ret = sqlite3_open(SUBSURFACE_TEST_DATA "/dives/TestDivingLog4.1.1.sql", &_sqlite3_handle);