core: convert uemis.c to C++

The uemis code is wild. It simply doesn't deallocate memory
and uses global variables. To get this under control, create
a "struct uemis" and make the functions exported by "uemis.h"
members of "struct uemis". Thus, we don't have to carry around
a parameter for the state of the importing process.

Turn a linked list of "helper" structures (one per imported dive)
into a std::unordered_map, to fix leaking of the helper structures.

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

Subsurface-mobile.pro

@@ -99,7 +99,7 @@ SOURCES += subsurface-mobile-main.cpp \
 	core/time.cpp \
 	core/trip.c \
 	core/units.c \
-	core/uemis.c \
+	core/uemis.cpp \
 	core/btdiscovery.cpp \
 	core/connectionlistmodel.cpp \
 	core/qt-ble.cpp \

core/CMakeLists.txt

@@ -187,7 +187,7 @@ set(SUBSURFACE_CORE_LIB_SRCS
 	trip.c
 	trip.h
 	uemis-downloader.cpp
-	uemis.c
+	uemis.cpp
 	uemis.h
 	units.h
 	units.c

core/uemis-downloader.cpp

@@ -26,13 +26,14 @@
 #include "gettext.h"
 #include "libdivecomputer.h"
 #include "uemis.h"
+#include "dive.h"
 #include "divelist.h"
 #include "divelog.h"
 #include "divesite.h"
 #include "errorhelper.h"
 #include "file.h"
-#include "tag.h"
 #include "subsurface-time.h"
+#include "tag.h"
 #include "core/qthelper.h"
 #include "core/subsurface-string.h"
 
@@ -74,6 +75,7 @@ static int debug_round = 0;
 #define UEMIS_MAX_TIMEOUT 2000000	/* 2s */
 #endif
 
+static uemis uemis_obj;
 static const char *param_buff[NUM_PARAM_BUFS];
 static std::string reqtxt_path;
 static int reqtxt_file;
@@ -199,7 +201,7 @@ static void uemis_add_string(const char *buffer, char **text, const char *delimi
 /* still unclear if it ever reports lbs */
 static void uemis_get_weight(char *buffer, weightsystem_t *weight, int diveid)
 {
-	weight->weight.grams = uemis_get_weight_unit(diveid) ?
+	weight->weight.grams = uemis_obj.get_weight_unit(diveid) ?
 		lbs_to_grams(ascii_strtod(buffer, NULL)) :
 		lrint(ascii_strtod(buffer, NULL) * 1000);
 	weight->description = translate("gettextFromC", "unknown");
@@ -790,7 +792,7 @@ static bool parse_divespot(char *buf)
 		}
 	} while (tag && *tag);
 
-	uemis_set_divelocation(divespot, locationstring, longitude, latitude);
+	uemis_obj.set_divelocation(divespot, locationstring, longitude, latitude);
 	return true;
 }
 
@@ -813,7 +815,7 @@ static void parse_tag(struct dive *dive, char *tag, char *val)
 	} else if (!strcmp(tag, "depth")) {
 		uemis_depth(val, &dive->dc.maxdepth);
 	} else if (!strcmp(tag, "file_content")) {
-		uemis_parse_divelog_binary(val, dive);
+		uemis_obj.parse_divelog_binary(val, dive);
 	} else if (!strcmp(tag, "altitude")) {
 		uemis_get_index(val, &dive->dc.surface_pressure.mbar);
 	} else if (!strcmp(tag, "f32Weight")) {
@@ -996,7 +998,7 @@ static bool process_raw_buffer(device_data_t *devdata, uint32_t deviceid, char *
 				struct dive_site *ds = create_dive_site("from Uemis", devdata->log->sites);
 				unregister_dive_from_dive_site(dive);
 				add_dive_to_dive_site(dive, ds);
-				uemis_mark_divelocation(dive->dc.diveid, divespot_id, ds);
+				uemis_obj.mark_divelocation(dive->dc.diveid, divespot_id, ds);
 			}
 #if UEMIS_DEBUG & 2
 			fprintf(debugfile, "Created divesite %d for diveid : %d\n", dive->dive_site->uuid, dive->dc.diveid);
@@ -1256,7 +1258,7 @@ static bool get_matching_dive(int idx, char *newmax, int *uemis_mem_status, devi
 						d_time = get_dive_date_c_string(dive->when);
 						fprintf(debugfile, "Matching dive log id %d from %s with dive details %d\n", dive->dc.diveid, d_time.c_str(), dive_to_read);
 #endif
-						int divespot_id = uemis_get_divespot_id_by_diveid(dive->dc.diveid);
+						int divespot_id = uemis_obj.get_divespot_id_by_diveid(dive->dc.diveid);
 						if (divespot_id >= 0)
 							get_uemis_divespot(data, mountpath, divespot_id, dive);
 

core/uemis.cpp

@@ -14,12 +14,43 @@
 #include "gettext.h"
 
 #include "uemis.h"
+#include "dive.h"
+#include "divecomputer.h"
 #include "divesite.h"
 #include "errorhelper.h"
 #include "sample.h"
 #include <libdivecomputer/parser.h>
 #include <libdivecomputer/version.h>
 
+struct uemis_sample
+{
+	uint16_t dive_time;
+	uint16_t water_pressure;   // (in cbar)
+	uint16_t dive_temperature; // (in dC)
+	uint8_t ascent_speed;      // (units unclear)
+	uint8_t work_fact;
+	uint8_t cold_fact;
+	uint8_t bubble_fact;
+	uint16_t ascent_time;
+	uint16_t ascent_time_opt;
+	uint16_t p_amb_tol;
+	uint16_t satt;
+	uint16_t hold_depth;
+	uint16_t hold_time;
+	uint8_t active_tank;
+	// bloody glib, when compiled for Windows, forces the whole program to use
+	// the Windows packing rules. So to avoid problems on Windows (and since
+	// only tank_pressure is currently used and that exactly once) I give in and
+	// make this silly low byte / high byte 8bit entries
+	uint8_t tank_pressure_low; // (in cbar)
+	uint8_t tank_pressure_high;
+	uint8_t consumption_low; // (units unclear)
+	uint8_t consumption_high;
+	uint8_t rgt; // (remaining gas time in minutes)
+	uint8_t cns;
+	uint8_t flags[8];
+} __attribute((packed));
+
 /*
  * following code is based on code found in at base64.sourceforge.net/b64.c
  * AUTHOR:         Bob Trower 08/04/01
@@ -78,7 +109,7 @@ static void decode(uint8_t *inbuf, uint8_t *outbuf, int inbuf_len)
 /*
  * convert the base64 data blog
  */
-static int uemis_convert_base64(char *base64, uint8_t **data)
+static std::vector<uint8_t> convert_base64(char *base64)
 {
 	int len, datalen;
 
@@ -88,99 +119,57 @@ static int uemis_convert_base64(char *base64, uint8_t **data)
 		/* less than header + 1 sample??? */
 		report_info("suspiciously short data block %d", datalen);
 
-	*data = malloc(datalen);
+	std::vector<uint8_t> res(datalen);
-	if (!*data) {
+	decode((unsigned char *)base64, res.data(), len);
-		fprintf(stderr, "Out of memory\n");
-		return 0;
-	}
-	decode((unsigned char *)base64, *data, len);
 
-	if (memcmp(*data, "Dive\01\00\00", 7))
+	if (memcmp(res.data(), "Dive\01\00\00", 7))
 		report_info("Missing Dive100 header");
 
-	return datalen;
+	return res;
 }
 
-struct uemis_helper {
+struct uemis::helper &uemis::get_helper(uint32_t diveid)
-	uint32_t diveid;
-	int lbs;
-	int divespot;
-	struct dive_site *dive_site;
-	struct uemis_helper *next;
-};
-static struct uemis_helper *uemis_helper = NULL;
-
-static struct uemis_helper *uemis_get_helper(uint32_t diveid)
 {
-	struct uemis_helper **php = &uemis_helper;
+	return helper_table[diveid];
-	struct uemis_helper *hp = *php;
-
-	while (hp) {
-		if (hp->diveid == diveid)
-			return hp;
-		if (hp->next) {
-			hp = hp->next;
-			continue;
-		}
-		php = &hp->next;
-		break;
-	}
-	hp = *php = calloc(1, sizeof(struct uemis_helper));
-	hp->diveid = diveid;
-	hp->next = NULL;
-	return hp;
 }
 
-static void uemis_weight_unit(int diveid, int lbs)
+void uemis::weight_unit(int diveid, int lbs)
 {
-	struct uemis_helper *hp = uemis_get_helper(diveid);
+	struct uemis::helper &hp = get_helper(diveid);
-	if (hp)
+	hp.lbs = lbs;
-		hp->lbs = lbs;
 }
 
-int uemis_get_weight_unit(uint32_t diveid)
+int uemis::get_weight_unit(uint32_t diveid) const
 {
-	struct uemis_helper *hp = uemis_helper;
+	auto it = helper_table.find(diveid);
-	while (hp) {
+	return it != helper_table.end() ? it->second.lbs : 0;
-		if (hp->diveid == diveid)
-			return hp->lbs;
-		hp = hp->next;
-	}
-	/* odd - we should have found this; default to kg */
-	return 0;
 }
 
-void uemis_mark_divelocation(int diveid, int divespot, struct dive_site *ds)
+void uemis::mark_divelocation(int diveid, int divespot, struct dive_site *ds)
 {
-	struct uemis_helper *hp = uemis_get_helper(diveid);
+	struct uemis::helper &hp = get_helper(diveid);
-	hp->divespot = divespot;
+	hp.divespot = divespot;
-	hp->dive_site = ds;
+	hp.dive_site = ds;
 }
 
 /* support finding a dive spot based on the diveid */
-int uemis_get_divespot_id_by_diveid(uint32_t diveid)
+int uemis::get_divespot_id_by_diveid(uint32_t diveid) const
 {
-	struct uemis_helper *hp = uemis_helper;
+	auto it = helper_table.find(diveid);
-	while (hp) {
+	return it != helper_table.end() ? it->second.divespot : -1;
-		if (hp->diveid == diveid)
-			return hp->divespot;
-		hp = hp->next;
-	}
-	return -1;
 }
 
-void uemis_set_divelocation(int divespot, char *text, double longitude, double latitude)
+void uemis::set_divelocation(int divespot, char *text, double longitude, double latitude)
 {
-	struct uemis_helper *hp = uemis_helper;
+	for (auto it: helper_table) {
-	while (hp) {
+		if (it.second.divespot == divespot) {
-		if (hp->divespot == divespot) {
+			struct dive_site *ds = it.second.dive_site;
-			struct dive_site *ds = hp->dive_site;
 			if (ds) {
+				free(ds->name);
 				ds->name = strdup(text);
 				ds->location = create_location(latitude, longitude);
 			}
 		}
-		hp = hp->next;
 	}
 }
 
@@ -194,9 +183,9 @@ void uemis_set_divelocation(int divespot, char *text, double longitude, double l
  * when we write them to the XML file we'll always have the English strings,
  * regardless of locale
  */
-static void uemis_event(struct dive *dive, struct divecomputer *dc, struct sample *sample, uemis_sample_t *u_sample)
+void uemis::event(struct dive *dive, struct divecomputer *dc, struct sample *sample, const uemis_sample *u_sample)
 {
-	uint8_t *flags = u_sample->flags;
+	const uint8_t *flags = u_sample->flags;
 	int stopdepth;
 	static int lastndl;
 
@@ -290,32 +279,29 @@ static void uemis_event(struct dive *dive, struct divecomputer *dc, struct sampl
 /*
  * parse uemis base64 data blob into struct dive
  */
-void uemis_parse_divelog_binary(char *base64, struct dive *dive)
+void uemis::parse_divelog_binary(char *base64, struct dive *dive)
 {
-	int datalen;
-	int i;
-	uint8_t *data;
 	struct sample *sample = NULL;
-	uemis_sample_t *u_sample;
+	uemis_sample *u_sample;
 	struct divecomputer *dc = &dive->dc;
-	int template, gasoffset;
+	int dive_template, gasoffset;
 	uint8_t active = 0;
 
-	datalen = uemis_convert_base64(base64, &data);
+	auto data = convert_base64(base64);
-	dive->dc.airtemp.mkelvin = C_to_mkelvin((*(uint16_t *)(data + 45)) / 10.0);
+	dive->dc.airtemp.mkelvin = C_to_mkelvin((*(uint16_t *)(data.data() + 45)) / 10.0);
-	dive->dc.surface_pressure.mbar = *(uint16_t *)(data + 43);
+	dive->dc.surface_pressure.mbar = *(uint16_t *)(data.data() + 43);
-	if (*(uint8_t *)(data + 19))
+	if (*(uint8_t *)(data.data() + 19))
 		dive->dc.salinity = SEAWATER_SALINITY; /* avg grams per 10l sea water */
 	else
 		dive->dc.salinity = FRESHWATER_SALINITY; /* grams per 10l fresh water */
 
 	/* this will allow us to find the last dive read so far from this computer */
 	dc->model = strdup("Uemis Zurich");
-	dc->deviceid = *(uint32_t *)(data + 9);
+	dc->deviceid = *(uint32_t *)(data.data() + 9);
-	dc->diveid = *(uint16_t *)(data + 7);
+	dc->diveid = *(uint16_t *)(data.data() + 7);
 	/* remember the weight units used in this dive - we may need this later when
 	 * parsing the weight */
-	uemis_weight_unit(dc->diveid, *(uint8_t *)(data + 24));
+	weight_unit(dc->diveid, *(uint8_t *)(data.data() + 24));
 	/* dive template in use:
 	   0 = air
 	   1 = nitrox (B)
@@ -324,13 +310,13 @@ void uemis_parse_divelog_binary(char *base64, struct dive *dive)
 	   uemis cylinder data is insane - it stores seven tank settings in a block
 	   and the template tells us which of the four groups of tanks we need to look at
 	 */
-	gasoffset = template = *(uint8_t *)(data + 115);
+	gasoffset = dive_template = *(uint8_t *)(data.data() + 115);
-	if (template == 3)
+	if (dive_template == 3)
 		gasoffset = 4;
-	if (template == 0)
+	if (dive_template == 0)
-		template = 1;
+		dive_template = 1;
-	for (i = 0; i < template; i++) {
+	for (int i = 0; i < dive_template; i++) {
-		float volume = *(float *)(data + 116 + 25 * (gasoffset + i)) * 1000.0f;
+		float volume = *(float *)(data.data() + 116 + 25 * (gasoffset + i)) * 1000.0f;
 		/* uemis always assumes a working pressure of 202.6bar (!?!?) - I first thought
 		 * it was 3000psi, but testing against all my dives gets me that strange number.
 		 * Still, that's of course completely bogus and shows they don't get how
@@ -341,13 +327,13 @@ void uemis_parse_divelog_binary(char *base64, struct dive *dive)
 		cylinder_t *cyl = get_or_create_cylinder(dive, i);
 		cyl->type.size.mliter = lrintf(volume);
 		cyl->type.workingpressure.mbar = 202600;
-		cyl->gasmix.o2.permille = *(uint8_t *)(data + 120 + 25 * (gasoffset + i)) * 10;
+		cyl->gasmix.o2.permille = *(uint8_t *)(data.data() + 120 + 25 * (gasoffset + i)) * 10;
 		cyl->gasmix.he.permille = 0;
 	}
 	/* first byte of divelog data is at offset 0x123 */
-	i = 0x123;
+	size_t i = 0x123;
-	u_sample = (uemis_sample_t *)(data + i);
+	u_sample = (uemis_sample *)(data.data() + i);
-	while ((i <= datalen) && (data[i] != 0 || data[i + 1] != 0)) {
+	while ((i <= data.size()) && (data[i] != 0 || data[i + 1] != 0)) {
 		if (u_sample->active_tank != active) {
 			if (u_sample->active_tank >= dive->cylinders.nr) {
 				report_info("got invalid sensor #%d was #%d", u_sample->active_tank, active);
@@ -362,7 +348,7 @@ void uemis_parse_divelog_binary(char *base64, struct dive *dive)
 		sample->temperature.mkelvin = C_to_mkelvin(u_sample->dive_temperature / 10.0);
 		add_sample_pressure(sample, active, (u_sample->tank_pressure_high * 256 + u_sample->tank_pressure_low) * 10);
 		sample->cns = u_sample->cns;
-		uemis_event(dive, dc, sample, u_sample);
+		event(dive, dc, sample, u_sample);
 		finish_sample(dc);
 		i += 0x25;
 		u_sample++;
@@ -375,17 +361,17 @@ void uemis_parse_divelog_binary(char *base64, struct dive *dive)
 
 	snprintf(buffer, sizeof(buffer), "%1u.%02u", data[18], data[17]);
 	add_extra_data(dc, "FW Version", buffer);
-	snprintf(buffer, sizeof(buffer), "%08x", *(uint32_t *)(data + 9));
+	snprintf(buffer, sizeof(buffer), "%08x", *(uint32_t *)(data.data() + 9));
 	add_extra_data(dc, "Serial", buffer);
-	snprintf(buffer, sizeof(buffer), "%d", *(uint16_t *)(data + i + 35));
+	snprintf(buffer, sizeof(buffer), "%d", *(uint16_t *)(data.data() + i + 35));
 	add_extra_data(dc, "main battery after dive", buffer);
-	snprintf(buffer, sizeof(buffer), "%0u:%02u", FRACTION_TUPLE(*(uint16_t *)(data + i + 24), 60));
+	snprintf(buffer, sizeof(buffer), "%0u:%02u", FRACTION_TUPLE(*(uint16_t *)(data.data() + i + 24), 60));
 	add_extra_data(dc, "no fly time", buffer);
-	snprintf(buffer, sizeof(buffer), "%0u:%02u", FRACTION_TUPLE(*(uint16_t *)(data + i + 26), 60));
+	snprintf(buffer, sizeof(buffer), "%0u:%02u", FRACTION_TUPLE(*(uint16_t *)(data.data() + i + 26), 60));
 	add_extra_data(dc, "no dive time", buffer);
-	snprintf(buffer, sizeof(buffer), "%0u:%02u", FRACTION_TUPLE(*(uint16_t *)(data + i + 28), 60));
+	snprintf(buffer, sizeof(buffer), "%0u:%02u", FRACTION_TUPLE(*(uint16_t *)(data.data() + i + 28), 60));
 	add_extra_data(dc, "desat time", buffer);
-	snprintf(buffer, sizeof(buffer), "%u", *(uint16_t *)(data + i + 30));
+	snprintf(buffer, sizeof(buffer), "%u", *(uint16_t *)(data.data() + i + 30));
 	add_extra_data(dc, "allowed altitude", buffer);
 
 	return;

core/uemis.h

@@ -6,50 +6,38 @@
 #ifndef UEMIS_H
 #define UEMIS_H
 
-#include <stdint.h>
-#include "dive.h"
 
 #ifdef __cplusplus
-extern "C" {
-#endif
 
-void uemis_parse_divelog_binary(char *base64, struct dive *dive);
+#include <unordered_map>
-int uemis_get_weight_unit(uint32_t diveid);
+#include <cstdint>
-void uemis_mark_divelocation(int diveid, int divespot, struct dive_site *ds);
+
-void uemis_set_divelocation(int divespot, char *text, double longitude, double latitude);
+struct dive;
-int uemis_get_divespot_id_by_diveid(uint32_t diveid);
+struct uemis_sample;
+struct dive_site;
+
+struct uemis {
+	void parse_divelog_binary(char *base64, struct dive *dive);
+	int get_weight_unit(uint32_t diveid) const;
+	void mark_divelocation(int diveid, int divespot, struct dive_site *ds);
+	void set_divelocation(int divespot, char *text, double longitude, double latitude);
+	int get_divespot_id_by_diveid(uint32_t diveid) const;
+private:
+	struct helper {
+		int lbs = 9;
+		int divespot = 9;
+		struct dive_site *dive_site = nullptr;
+	};
+	// Use a hash-table (std::unordered_map) to access dive information.
+	// Might also use a balanced binary tree (std::map) or a sorted array (std::vector).
+	std::unordered_map<uint32_t, helper> helper_table;
+
+	static void event(struct dive *dive, struct divecomputer *dc, struct sample *sample, const uemis_sample *u_sample);
+	struct helper &get_helper(uint32_t diveid);
+	void weight_unit(int diveid, int lbs);
+};
 
-typedef struct
-{
-	uint16_t dive_time;
-	uint16_t water_pressure;   // (in cbar)
-	uint16_t dive_temperature; // (in dC)
-	uint8_t ascent_speed;      // (units unclear)
-	uint8_t work_fact;
-	uint8_t cold_fact;
-	uint8_t bubble_fact;
-	uint16_t ascent_time;
-	uint16_t ascent_time_opt;
-	uint16_t p_amb_tol;
-	uint16_t satt;
-	uint16_t hold_depth;
-	uint16_t hold_time;
-	uint8_t active_tank;
-	// bloody glib, when compiled for Windows, forces the whole program to use
-	// the Windows packing rules. So to avoid problems on Windows (and since
-	// only tank_pressure is currently used and that exactly once) I give in and
-	// make this silly low byte / high byte 8bit entries
-	uint8_t tank_pressure_low; // (in cbar)
-	uint8_t tank_pressure_high;
-	uint8_t consumption_low; // (units unclear)
-	uint8_t consumption_high;
-	uint8_t rgt; // (remaining gas time in minutes)
-	uint8_t cns;
-	uint8_t flags[8];
-} __attribute((packed)) uemis_sample_t;
 
-#ifdef __cplusplus
-}
 #endif
 
 #endif // UEMIS_H

desktop-widgets/downloadfromdivecomputer.cpp

@@ -9,7 +9,6 @@
 #include "core/settings/qPrefDiveComputer.h"
 #include "core/subsurface-float.h"
 #include "core/subsurface-string.h"
-#include "core/uemis.h"
 #include "core/downloadfromdcthread.h"
 #include "desktop-widgets/divelistview.h"
 #include "desktop-widgets/mainwindow.h"