Import: keep dive and dive site tables in DiveImportedModel

The DiveImportedModel and DownloadThread used the same table
of dives and dive sites. This made it very hard to keep the
model consistent: Every modification of the download thread
would make the model inconsistent and could lead to memory
corruption owing to dangling pointers.

Therefore, keep a copy in the model. When updating the model,
use move-semantics, i.e. move the data and reset the tables
of the thread to zero elements.

Since the DiveImportedModel and the DownloadThread are very
tightly integrated, remove the accessor-functions of the
dive and dive-site tables. They fulfilled no purpose
whatsoever as they gave the same access-rights as a public
field.

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

core/downloadfromdcthread.cpp

@@ -308,16 +308,6 @@ DCDeviceData *DownloadThread::data()
 	return m_data;
 }
 
-struct dive_table *DownloadThread::table()
-{
-	return &downloadTable;
-}
-
-struct dive_site_table *DownloadThread::sites()
-{
-	return &diveSiteTable;
-}
-
 QString DCDeviceData::vendor() const
 {
 	return data.vendor;

core/downloadfromdcthread.h

@@ -64,13 +64,11 @@ public:
 	void run() override;
 
 	DCDeviceData *data();
-	struct dive_table *table();
-	struct dive_site_table *sites();
 	QString error;
-
-private:
 	struct dive_table downloadTable;
 	struct dive_site_table diveSiteTable;
+
+private:
 	DCDeviceData *m_data;
 };
 

qt-models/diveimportedmodel.cpp

@@ -5,8 +5,8 @@
 DiveImportedModel::DiveImportedModel(QObject *o) : QAbstractTableModel(o),
 	firstIndex(0),
 	lastIndex(-1),
-	diveTable(nullptr),
-	sitesTable(nullptr)
+	diveTable({ 0 }),
+	sitesTable({ 0 })
 {
 	connect(&thread, &QThread::finished, this, &DiveImportedModel::downloadThreadFinished);
 }
@@ -54,7 +54,7 @@ QVariant DiveImportedModel::data(const QModelIndex &index, int role) const
 	if (index.row() + firstIndex > lastIndex)
 		return QVariant();
 
-	struct dive *d = get_dive_from_table(index.row() + firstIndex, diveTable);
+	struct dive *d = get_dive_from_table(index.row() + firstIndex, &diveTable);
 	if (!d)
 		return QVariant();
 
@@ -132,7 +132,19 @@ void DiveImportedModel::clearTable()
 
 void DiveImportedModel::downloadThreadFinished()
 {
-	repopulate(thread.table(), thread.sites());
+	beginResetModel();
+
+	// Move the table data from thread to model
+	move_dive_table(&thread.downloadTable, &diveTable);
+	move_dive_site_table(&thread.diveSiteTable, &sitesTable);
+
+	firstIndex = 0;
+	lastIndex = diveTable.nr - 1;
+	checkStates.resize(diveTable.nr);
+	std::fill(checkStates.begin(), checkStates.end(), true);
+
+	endResetModel();
+
 	emit downloadFinished();
 }
 
@@ -141,29 +153,15 @@ void DiveImportedModel::startDownload()
 	thread.start();
 }
 
-void DiveImportedModel::repopulate(dive_table_t *table, struct dive_site_table *sites)
-{
-	beginResetModel();
-
-	diveTable = table;
-	sitesTable = sites;
-	firstIndex = 0;
-	lastIndex = diveTable->nr - 1;
-	checkStates.resize(diveTable->nr);
-	std::fill(checkStates.begin(), checkStates.end(), true);
-
-	endResetModel();
-}
-
 std::pair<struct dive_table, struct dive_site_table> DiveImportedModel::consumeTables()
 {
 	beginResetModel();
 
 	// Move tables to result
-	struct dive_table dives;
-	struct dive_site_table sites;
-	move_dive_table(diveTable, &dives);
-	move_dive_site_table(sitesTable, &sites);
+	struct dive_table dives = { 0 };
+	struct dive_site_table sites = { 0 };
+	move_dive_table(&diveTable, &dives);
+	move_dive_site_table(&sitesTable, &sites);
 
 	// Reset indexes
 	firstIndex = 0;
@@ -177,38 +175,38 @@ std::pair<struct dive_table, struct dive_site_table> DiveImportedModel::consumeT
 
 int DiveImportedModel::numDives() const
 {
-	return diveTable->nr;
+	return diveTable.nr;
 }
 
 // Delete non-selected dives
 void DiveImportedModel::deleteDeselected()
 {
-	int total = diveTable->nr;
+	int total = diveTable.nr;
 	int j = 0;
 	for (int i = 0; i < total; i++) {
 		if (checkStates[i]) {
 			j++;
 		} else {
 			beginRemoveRows(QModelIndex(), j, j);
-			delete_dive_from_table(diveTable, j);
+			delete_dive_from_table(&diveTable, j);
 			endRemoveRows();
 		}
 	}
-	checkStates.resize(diveTable->nr);
+	checkStates.resize(diveTable.nr);
 	std::fill(checkStates.begin(), checkStates.end(), true);
 }
 
 // Note: this function is only used from mobile - perhaps move it there or unify.
 void DiveImportedModel::recordDives()
 {
-	if (diveTable->nr == 0)
+	if (diveTable.nr == 0)
 		// nothing to do, just exit
 		return;
 
 	deleteDeselected();
 
 	// TODO: Might want to let the user select IMPORT_ADD_TO_NEW_TRIP
-	add_imported_dives(diveTable, nullptr, sitesTable, IMPORT_PREFER_IMPORTED | IMPORT_IS_DOWNLOADED);
+	add_imported_dives(&diveTable, nullptr, &sitesTable, IMPORT_PREFER_IMPORTED | IMPORT_IS_DOWNLOADED);
 }
 
 QHash<int, QByteArray> DiveImportedModel::roleNames() const {

qt-models/diveimportedmodel.h

@@ -23,6 +23,7 @@ public:
 	QHash<int, QByteArray> roleNames() const;
 	void deleteDeselected();
 	std::pair<struct dive_table, struct dive_site_table> consumeTables(); // Returns dives and sites and resets model.
+
 	int numDives() const;
 	Q_INVOKABLE void recordDives();
 	Q_INVOKABLE void startDownload();
@@ -43,12 +44,11 @@ signals:
 	void downloadFinished();
 
 private:
-	void repopulate(dive_table_t *table, dive_site_table_t *sites);
 	int firstIndex;
 	int lastIndex;
 	std::vector<char> checkStates; // char instead of bool to avoid silly pessimization of std::vector.
-	struct dive_table *diveTable;
-	struct dive_site_table *sitesTable;
+	struct dive_table diveTable;
+	struct dive_site_table sitesTable;
 };
 
 #endif