Make sure we don't create multiple dive sites with the same uuid

This shouldn't happen, but in case there is a logic error higher up in the
code somewhere, this will prevent it from happening, period.

If the code asks for a new dive site with a specific uuid, simply return
the existing dive site with that uuid.

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

5 files changed, 28 insertions, 14 deletions

diff --git a/divesite.c b/divesite.c
index 1e179820f..e9eed2a07 100644
--- a/divesite.c
+++ b/divesite.c
@@ -101,9 +101,18 @@ static uint32_t dive_site_getUniqId()
 	return id;
 }
 
-struct dive_site *alloc_dive_site(uint32_t uuid)
+/* we never allow a second dive site with the same uuid */
+struct dive_site *alloc_or_get_dive_site(uint32_t uuid)
 {
-	int nr = dive_site_table.nr, allocated = dive_site_table.allocated;
+	struct dive_site *ds;
+	if (uuid) {
+		if ((ds = get_dive_site_by_uuid(uuid)) != NULL) {
+			fprintf(stderr, "PROBLEM: refusing to create dive site with the same uuid %08x\n", uuid);
+			return ds;
+		}
+	}
+	int nr = dive_site_table.nr;
+	int allocated = dive_site_table.allocated;
 	struct dive_site **sites = dive_site_table.dive_sites;
 
 	if (nr >= allocated) {
@@ -114,18 +123,19 @@ struct dive_site *alloc_dive_site(uint32_t uuid)
 		dive_site_table.dive_sites = sites;
 		dive_site_table.allocated = allocated;
 	}
-	struct dive_site *ds = calloc(1, sizeof(*ds));
+	ds = calloc(1, sizeof(*ds));
 	if (!ds)
 		exit(1);
 	sites[nr] = ds;
 	dive_site_table.nr = nr + 1;
-	if (uuid) {
-		if (get_dive_site_by_uuid(uuid))
-			fprintf(stderr, "PROBLEM: duplicate uuid %08x\n", uuid);
+	// we should always be called with a valid uuid except in the special
+	// case where we want to copy a dive site into the memory we allocated
+	// here - then we need to pass in 0 and create a temporary uuid here
+	// (just so things are always consistent)
+	if (uuid)
 		ds->uuid = uuid;
-	} else {
+	else
 		ds->uuid = dive_site_getUniqId();
-	}
 	return ds;
 }
 
@@ -193,7 +203,7 @@ uint32_t create_divesite_uuid(const char *name, timestamp_t divetime)
 uint32_t create_dive_site(const char *name, timestamp_t divetime)
 {
 	uint32_t uuid = create_divesite_uuid(name, divetime);
-	struct dive_site *ds = alloc_dive_site(uuid);
+	struct dive_site *ds = alloc_or_get_dive_site(uuid);
 	ds->name = copy_string(name);
 
 	return uuid;
@@ -216,7 +226,7 @@ uint32_t create_dive_site_from_current_dive(const char *name)
 uint32_t create_dive_site_with_gps(const char *name, degrees_t latitude, degrees_t longitude, timestamp_t divetime)
 {
 	uint32_t uuid = create_divesite_uuid(name, divetime);
-	struct dive_site *ds = alloc_dive_site(uuid);
+	struct dive_site *ds = alloc_or_get_dive_site(uuid);
 	ds->name = copy_string(name);
 	ds->latitude = latitude;
 	ds->longitude = longitude;
diff --git a/divesite.h b/divesite.h
index 345b50448..d8987075b 100644
--- a/divesite.h
+++ b/divesite.h
@@ -50,7 +50,7 @@ static inline struct dive_site *get_dive_site_by_uuid(uint32_t uuid)
 }
 
 void dive_site_table_sort();
-struct dive_site *alloc_dive_site(uint32_t uuid);
+struct dive_site *alloc_or_get_dive_site(uint32_t uuid);
 int nr_of_dives_at_dive_site(uint32_t uuid, bool select_only);
 bool is_dive_site_used(uint32_t uuid, bool select_only);
 void delete_dive_site(uint32_t id);
diff --git a/load-git.c b/load-git.c
index d50a53d6c..8c7df7bcc 100644
--- a/load-git.c
+++ b/load-git.c
@@ -1444,7 +1444,7 @@ static int parse_site_entry(git_repository *repo, const git_tree_entry *entry, c
 	if (*suffix == '\0')
 		return report_error("Dive site without uuid");
 	uint32_t uuid = strtoul(suffix, NULL, 16);
-	struct dive_site *ds = alloc_dive_site(uuid);
+	struct dive_site *ds = alloc_or_get_dive_site(uuid);
 	git_blob *blob = git_tree_entry_blob(repo, entry);
 	if (!blob)
 		return report_error("Unable to read dive site file");
diff --git a/parse-xml.c b/parse-xml.c
index be5215509..9b223980d 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -1562,7 +1562,11 @@ static void dive_site_end(void)
 	if (!cur_dive_site)
 		return;
 	if (cur_dive_site->uuid) {
-		struct dive_site *ds = alloc_dive_site(0);
+		// we intentionally call this with '0' to ensure we get
+		// a new structure and then copy things into that new
+		// structure a few lines below (which sets the correct
+		// uuid)
+		struct dive_site *ds = alloc_or_get_dive_site(0);
 		if (cur_dive_site->taxonomy.nr == 0) {
 			free(cur_dive_site->taxonomy.category);
 			cur_dive_site->taxonomy.category = NULL;
diff --git a/tests/testparse.cpp b/tests/testparse.cpp
index 1a4c0eac5..7a65a0105 100644
--- a/tests/testparse.cpp
+++ b/tests/testparse.cpp
@@ -83,7 +83,7 @@ void TestParse::testParseDivingLog()
 	// Parsing of DivingLog import from SQLite database
 	sqlite3 *handle;
 
-	struct dive_site *ds = alloc_dive_site(0xdeadbeef);
+	struct dive_site *ds = alloc_or_get_dive_site(0xdeadbeef);
 	ds->name = copy_string("Suomi -  - Hälvälä");
 
 	QCOMPARE(sqlite3_open(SUBSURFACE_SOURCE "/dives/TestDivingLog4.1.1.sql", &handle), 0);