uemis: replace C-strings by std::string and std::string_view

The string code of uemis-downloader.cpp was broken in more ways
than can be listed here. Notably, it brazenly refused to free any
memory allocated for the parameters buffer.

Using std::string and std::string_view should plug all those
memory holes. That made it necessary to do some major refactoring.

This was done blind and therefore will break.

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

core/downloadfromdcthread.cpp

@@ -1,8 +1,10 @@
 #include "downloadfromdcthread.h"
 #include "core/errorhelper.h"
+#include "core/format.h"
 #include "core/libdivecomputer.h"
 #include "core/qthelper.h"
 #include "core/range.h"
+#include "core/uemis.h"
 #include "core/settings/qPrefDiveComputer.h"
 #include "core/divelist.h"
 #if defined(Q_OS_ANDROID)
@@ -15,16 +17,6 @@ static QHash<QString, QStringList> mobileProductList; // BT, BLE or FTDI support
 QMap<QString, dc_descriptor_t *> descriptorLookup;
 ConnectionListModel connectionListModel;
 
-static QString str_error(const char *fmt, ...)
-{
-	va_list args;
-	va_start(args, fmt);
-	const QString str = QString::vasprintf(fmt, args);
-	va_end(args);
-
-	return str;
-}
-
 static void updateRememberedDCs()
 {
 	QString current = qPrefDiveComputer::vendor() + " - " + qPrefDiveComputer::product() + " - " + qPrefDiveComputer::device();
@@ -108,19 +100,20 @@ void DownloadThread::run()
 	clear_divelog(&log);
 
 	Q_ASSERT(internalData->log != nullptr);
-	const char *errorText;
+	std::string errorText;
 	import_thread_cancelled = false;
 	error.clear();
 	if (!strcmp(internalData->vendor, "Uemis"))
 		errorText = do_uemis_import(internalData);
 	else
 		errorText = do_libdivecomputer_import(internalData);
-	if (errorText) {
-		error = str_error(errorText, internalData->devname, internalData->vendor, internalData->product);
-		report_info("Finishing download thread: %s", qPrintable(error));
+	if (!errorText.empty()) {
+		error = format_string_std(errorText.c_str(), internalData->devname.c_str(),
+					  internalData->vendor.c_str(), internalData->product.c_str());
+		report_info("Finishing download thread: %s", error.c_str());
 	} else {
 		if (!log.dives->nr)
-			error = tr("No new dives downloaded from dive computer");
+			error = tr("No new dives downloaded from dive computer").toStdString();
 		report_info("Finishing download thread: %d dives downloaded", log.dives->nr);
 	}
 	qPrefDiveComputer::set_vendor(internalData->vendor);

core/downloadfromdcthread.h

@@ -74,7 +74,7 @@ public:
 	void run() override;
 
 	DCDeviceData *data();
-	QString error;
+	std::string error;
 	struct divelog log;
 
 private:

core/libdivecomputer.cpp

@@ -1141,7 +1141,7 @@ static int cancel_cb(void *)
 	return import_thread_cancelled;
 }
 
-static const char *do_device_import(device_data_t *data)
+static std::string do_device_import(device_data_t *data)
 {
 	dc_status_t rc;
 	dc_device_t *device = data->device;
@@ -1202,7 +1202,7 @@ static const char *do_device_import(device_data_t *data)
 	}
 
 	/* All good */
-	return NULL;
+	return std::string();
 }
 
 static dc_timer_t *logfunc_timer = NULL;
@@ -1465,10 +1465,9 @@ static dc_status_t sync_divecomputer_time(dc_device_t *device)
 	return dc_device_timesync(device, &now);
 }
 
-const char *do_libdivecomputer_import(device_data_t *data)
+std::string do_libdivecomputer_import(device_data_t *data)
 {
 	dc_status_t rc;
-	const char *err;
 	FILE *fp = NULL;
 
 	import_dive_number = 0;
@@ -1495,7 +1494,7 @@ const char *do_libdivecomputer_import(device_data_t *data)
 		fprintf(data->libdc_logfile, "built with libdivecomputer v%s\n", dc_version(NULL));
 	}
 
-	err = translate("gettextFromC", "Unable to open %s %s (%s)");
+	std::string err = translate("gettextFromC", "Unable to open %s %s (%s)");
 
 	rc = divecomputer_device_open(data);
 
@@ -1518,7 +1517,7 @@ const char *do_libdivecomputer_import(device_data_t *data)
 			/* TODO: Show the logfile to the user on error. */
 			dev_info(data, "Import complete");
 
-			if (!err && data->sync_time) {
+			if (err.empty() && data->sync_time) {
 				dev_info(data, "Syncing dive computer time ...");
 				rc = sync_divecomputer_time(data->device);
 

core/libdivecomputer.h

@@ -52,8 +52,7 @@ typedef struct {
 } device_data_t;
 
 const char *errmsg (dc_status_t rc);
-const char *do_libdivecomputer_import(device_data_t *data);
-const char *do_uemis_import(device_data_t *data);
+std::string do_libdivecomputer_import(device_data_t *data);
 dc_status_t libdc_buffer_parser(struct dive *dive, device_data_t *data, unsigned char *buffer, int size);
 void logfunc(dc_context_t *context, dc_loglevel_t loglevel, const char *file, unsigned int line, const char *function, const char *msg, void *userdata);
 dc_descriptor_t *get_descriptor(dc_family_t type, unsigned int model);

core/subsurface-string.h

@@ -59,16 +59,17 @@ extern double ascii_strtod(const char *str, const char **ptr);
 }
 
 #include <string>
+#include <string_view>
 #include <vector>
 
 // Sadly, starts_with only with C++20!
-inline bool starts_with(const std::string &s, const char *s2)
+inline bool starts_with(std::string_view s, const char *s2)
 {
 	return s.rfind(s2, 0) == 0;
 }
 
 // Sadly, std::string::contains only with C++23!
-inline bool contains(const std::string &s, char c)
+inline bool contains(std::string_view s, char c)
 {
 	return s.find(c) != std::string::npos;
 }

core/uemis.cpp

@@ -109,18 +109,18 @@ static void decode(uint8_t *inbuf, uint8_t *outbuf, int inbuf_len)
 /*
  * convert the base64 data blog
  */
-static std::vector<uint8_t> convert_base64(char *base64)
+static std::vector<uint8_t> convert_base64(std::string_view base64)
 {
-	int len, datalen;
+	int datalen;
+	int len = (int)base64.size();
 
-	len = strlen(base64);
 	datalen = (len / 4 + 1) * 3;
 	if (datalen < 0x123 + 0x25)
 		/* less than header + 1 sample??? */
 		report_info("suspiciously short data block %d", datalen);
 
 	std::vector<uint8_t> res(datalen);
-	decode((unsigned char *)base64, res.data(), len);
+	decode((unsigned char *)base64.begin(), res.data(), len);
 
 	if (memcmp(res.data(), "Dive\01\00\00", 7))
 		report_info("Missing Dive100 header");
@@ -159,14 +159,14 @@ int uemis::get_divespot_id_by_diveid(uint32_t diveid) const
 	return it != helper_table.end() ? it->second.divespot : -1;
 }
 
-void uemis::set_divelocation(int divespot, char *text, double longitude, double latitude)
+void uemis::set_divelocation(int divespot, const std::string &text, double longitude, double latitude)
 {
 	for (auto it: helper_table) {
 		if (it.second.divespot == divespot) {
 			struct dive_site *ds = it.second.dive_site;
 			if (ds) {
 				free(ds->name);
-				ds->name = strdup(text);
+				ds->name = strdup(text.c_str());
 				ds->location = create_location(latitude, longitude);
 			}
 		}
@@ -279,7 +279,7 @@ void uemis::event(struct dive *dive, struct divecomputer *dc, struct sample *sam
 /*
  * parse uemis base64 data blob into struct dive
  */
-void uemis::parse_divelog_binary(char *base64, struct dive *dive)
+void uemis::parse_divelog_binary(std::string_view base64, struct dive *dive)
 {
 	struct sample *sample = NULL;
 	uemis_sample *u_sample;

core/uemis.h

@@ -6,9 +6,12 @@
 #ifndef UEMIS_H
 #define UEMIS_H
 
+#include "libdivecomputer.h" // for device_data_t, which is a typedef, not a struct :(
 
 #ifdef __cplusplus
 
+#include <string>
+#include <string_view>
 #include <unordered_map>
 #include <cstdint>
 
@@ -17,10 +20,10 @@ struct uemis_sample;
 struct dive_site;
 
 struct uemis {
-	void parse_divelog_binary(char *base64, struct dive *dive);
+	void parse_divelog_binary(std::string_view 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);
+	void set_divelocation(int divespot, const std::string &text, double longitude, double latitude);
 	int get_divespot_id_by_diveid(uint32_t diveid) const;
 private:
 	struct helper {
@@ -37,6 +40,7 @@ private:
 	void weight_unit(int diveid, int lbs);
 };
 
+std::string do_uemis_import(device_data_t *data);
 
 #endif
 

desktop-widgets/downloadfromdivecomputer.cpp

@@ -330,7 +330,8 @@ void DownloadFromDCWidget::updateState(states state)
 	else if (state == ERRORED) {
 		timer->stop();
 
-		QMessageBox::critical(this, TITLE_OR_TEXT(tr("Error"), diveImportedModel->thread.error), QMessageBox::Ok);
+		QMessageBox::critical(this, TITLE_OR_TEXT(tr("Error"),
+					QString::fromStdString(diveImportedModel->thread.error)), QMessageBox::Ok);
 		markChildrenAsEnabled();
 		progress_bar_text = "";
 		ui.progressBar->hide();
@@ -533,7 +534,7 @@ void DownloadFromDCWidget::onDownloadThreadFinished()
 	showRememberedDCs();
 
 	if (currentState == DOWNLOADING) {
-		if (diveImportedModel->thread.error.isEmpty())
+		if (diveImportedModel->thread.error.empty())
 			updateState(DONE);
 		else
 			updateState(ERRORED);