From e20ec9248c1ae8c4adff3704f5fe57ddfd9e393a Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Date: Sun, 12 May 2024 12:46:57 +0200
Subject: [PATCH] core: fix leak when clearing dive log

46cf2fc0867 fixed a bug where clearing of a divelog, such as the one
used for import, would erase dives in the global(!) divelog.

However, the new code used the function clear_dive_table(), which
only cleared the table without unregistering the dives. In particular,
the dives were not removed from the trips, which means that the trips
were not free()d.

This reinstates the old code, but now passes a divelog paremeter
to delete_single_dive() instead of accessing the global divelog.
Moreover, delete dives from the back to avoid unnecessary
copying.

An alternative and definitely simpler solution might be to just
add a "clear_trip_table()" after "clear_dive_table()".

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
---
 core/divelist.c          | 14 +-------------
 core/divelist.h          |  1 -
 core/divelog.cpp         | 17 ++++++++++++++++-
 core/divelog.h           |  1 +
 tests/testgitstorage.cpp |  2 +-
 5 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/core/divelist.c b/core/divelist.c
index 30a4de16e..8c677498a 100644
--- a/core/divelist.c
+++ b/core/divelist.c
@@ -767,18 +767,6 @@ struct dive *unregister_dive(int idx)
 	return dive;
 }
 
-/* this implements the mechanics of removing the dive from the global
- * dive table and the trip, but doesn't deal with updating dive trips, etc */
-void delete_single_dive(int idx)
-{
-	struct dive *dive = get_dive(idx);
-	if (!dive)
-		return; /* this should never happen */
-	remove_dive_from_trip(dive, divelog.trips);
-	unregister_dive_from_dive_site(dive);
-	delete_dive_from_table(divelog.dives, idx);
-}
-
 void process_loaded_dives()
 {
 	sort_dive_table(divelog.dives);
@@ -989,7 +977,7 @@ void add_imported_dives(struct divelog *import_log, int flags)
 	/* Remove old dives */
 	for (i = 0; i < dives_to_remove.nr; i++) {
 		idx = get_divenr(dives_to_remove.dives[i]);
-		delete_single_dive(idx);
+		delete_single_dive(&divelog, idx);
 	}
 	dives_to_remove.nr = 0;
 
diff --git a/core/divelist.h b/core/divelist.h
index 62d9faad0..4b07ebf3b 100644
--- a/core/divelist.h
+++ b/core/divelist.h
@@ -62,7 +62,6 @@ void clear_dive_file_data();
 void clear_dive_table(struct dive_table *table);
 void move_dive_table(struct dive_table *src, struct dive_table *dst);
 struct dive *unregister_dive(int idx);
-extern void delete_single_dive(int idx);
 extern bool has_dive(unsigned int deviceid, unsigned int diveid);
 
 #ifdef __cplusplus
diff --git a/core/divelog.cpp b/core/divelog.cpp
index b25e461c6..f5c42b88c 100644
--- a/core/divelog.cpp
+++ b/core/divelog.cpp
@@ -64,9 +64,24 @@ struct divelog &divelog::operator=(divelog &&log)
 	return *this;
 }
 
+/* this implements the mechanics of removing the dive from the
+ * dive log and the trip, but doesn't deal with updating dive trips, etc */
+void delete_single_dive(struct divelog *log, int idx)
+{
+	if (idx < 0 || idx > log->dives->nr) {
+		report_info("Warning: deleting unexisting dive with index %d", idx);
+		return;
+	}
+	struct dive *dive = log->dives->dives[idx];
+	remove_dive_from_trip(dive, log->trips);
+	unregister_dive_from_dive_site(dive);
+	delete_dive_from_table(log->dives, idx);
+}
+
 void divelog::clear()
 {
-	clear_dive_table(dives);
+	while (dives->nr > 0)
+		delete_single_dive(this, dives->nr - 1);
 	while (sites->nr)
 		delete_dive_site(get_dive_site(0, sites), sites);
 	if (trips->nr != 0) {
diff --git a/core/divelog.h b/core/divelog.h
index c8d04ed6a..e1eb01770 100644
--- a/core/divelog.h
+++ b/core/divelog.h
@@ -34,6 +34,7 @@ extern "C" {
 #endif
 
 void clear_divelog(struct divelog *);
+extern void delete_single_dive(struct divelog *, int idx);
 
 #ifdef __cplusplus
 }
diff --git a/tests/testgitstorage.cpp b/tests/testgitstorage.cpp
index cdac7f1b3..7b1aafdfb 100644
--- a/tests/testgitstorage.cpp
+++ b/tests/testgitstorage.cpp
@@ -363,7 +363,7 @@ void TestGitStorage::testGitStorageCloudMerge2()
 	QCOMPARE(parse_file(cloudTestRepo.c_str(), &divelog), 0);
 	process_loaded_dives();
 	struct dive *dive = get_dive(1);
-	delete_single_dive(1);
+	delete_single_dive(&divelog, 1);
 	QCOMPARE(save_dives("./SampleDivesMinus1.ssrf"), 0);
 	git_local_only = true;
 	QCOMPARE(save_dives(localCacheRepo.c_str()), 0);