From fd8bd9d5c717799313c4decaa85796ba68449f8a Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Mon, 5 Oct 2020 09:56:21 +0200 Subject: cleanup: use std::string in struct device struct device is a core data structure and therefore shouldn't use QString. QString stores as UTF-16 (which is a very questionable choice in itself). However, the real problem is that this puts us in lifetime-management hell when interfacing with C code: The UTF-16 has to be converted to UTF-8, but when returning such a string, this puts burden on the caller who has to free it. In fact, instead of looping over devices from C-code we had a callback that sent down temporary C-strings with qPrintable. In contrast, std::string is guaranteed to store its data as contiguous null-terminated and C-compatible strings. Therefore, replace the QString by std::string. Keep the QString just in one place that formats a hexadecimal number to avoid any potential change. The disadvantage of using std::string is that it will crash when constructed with a NULL argument, consistent with C-style functions such as strcmp, etc. Arguably, NULL is different from the empty string even though we treat both as the same. Signed-off-by: Berthold Stoeger --- core/device.cpp | 54 ++++++++++++++++++++--------------------- core/device.h | 14 +++++------ qt-models/divecomputermodel.cpp | 6 ++--- 3 files changed, 37 insertions(+), 37 deletions(-) diff --git a/core/device.cpp b/core/device.cpp index 07ef0945f..39b3bc61b 100644 --- a/core/device.cpp +++ b/core/device.cpp @@ -2,11 +2,11 @@ #include "ssrf.h" #include "dive.h" #include "subsurface-string.h" -#include "qthelper.h" // for copy_qstring #include "device.h" #include "errorhelper.h" // for verbose flag #include "selection.h" #include "core/settings/qPrefDiveComputer.h" +#include // for QString::number /* * Good fake dive profiles are hard. @@ -238,29 +238,29 @@ extern "C" void set_dc_deviceid(struct divecomputer *dc, unsigned int deviceid) if (!node) return; - if (!node->serialNumber.isEmpty() && empty_string(dc->serial)) { + if (!node->serialNumber.empty() && empty_string(dc->serial)) { free((void *)dc->serial); - dc->serial = copy_qstring(node->serialNumber); + dc->serial = strdup(node->serialNumber.c_str()); } - if (!node->firmware.isEmpty() && empty_string(dc->fw_version)) { + if (!node->firmware.empty() && empty_string(dc->fw_version)) { free((void *)dc->fw_version); - dc->fw_version = copy_qstring(node->firmware); + dc->fw_version = strdup(node->firmware.c_str()); } } -void device::showchanges(const QString &n, const QString &s, const QString &f) const +void device::showchanges(const std::string &n, const std::string &s, const std::string &f) const { - if (nickName != n && !n.isEmpty()) - qDebug("new nickname %s for DC model %s deviceId 0x%x", qPrintable(n), qPrintable(model), deviceId); - if (serialNumber != s && !s.isEmpty()) - qDebug("new serial number %s for DC model %s deviceId 0x%x", qPrintable(s), qPrintable(model), deviceId); - if (firmware != f && !f.isEmpty()) - qDebug("new firmware version %s for DC model %s deviceId 0x%x", qPrintable(f), qPrintable(model), deviceId); + if (nickName != n && !n.empty()) + qDebug("new nickname %s for DC model %s deviceId 0x%x", n.c_str(), model.c_str(), deviceId); + if (serialNumber != s && !s.empty()) + qDebug("new serial number %s for DC model %s deviceId 0x%x", s.c_str(), model.c_str(), deviceId); + if (firmware != f && !f.empty()) + qDebug("new firmware version %s for DC model %s deviceId 0x%x", f.c_str(), model.c_str(), deviceId); } -static void addDC(QVector &dcs, const QString &m, uint32_t d, const QString &n, const QString &s, const QString &f) +static void addDC(QVector &dcs, const std::string &m, uint32_t d, const std::string &n, const std::string &s, const std::string &f) { - if (m.isEmpty() || d == 0) + if (m.empty() || d == 0) return; auto it = std::lower_bound(dcs.begin(), dcs.end(), device{m, d, {}, {}, {}}); if (it != dcs.end() && it->model == m && it->deviceId == d) { @@ -268,11 +268,11 @@ static void addDC(QVector &dcs, const QString &m, uint32_t d, const QStr if (verbose) it->showchanges(n, s, f); // Update any non-existent fields from the old entry - if (!n.isEmpty()) + if (!n.empty()) it->nickName = n; - if (!s.isEmpty()) + if (!s.empty()) it->serialNumber = s; - if (!f.isEmpty()) + if (!f.empty()) it->firmware = f; } else { dcs.insert(it, device{m, d, s, f, n}); @@ -281,7 +281,7 @@ static void addDC(QVector &dcs, const QString &m, uint32_t d, const QStr extern "C" void create_device_node(const char *model, uint32_t deviceid, const char *serial, const char *firmware, const char *nickname) { - addDC(device_table.devices, model, deviceid, nickname, serial, firmware); + addDC(device_table.devices, model ?: "", deviceid, nickname ?: "", serial ?: "", firmware ?: ""); } extern "C" void clear_device_nodes() @@ -310,8 +310,8 @@ extern "C" void call_for_each_dc (void *f, void (*callback)(void *, const char * found = true; } if (found) - callback(f, qPrintable(node.model), node.deviceId, qPrintable(node.nickName), - qPrintable(node.serialNumber), qPrintable(node.firmware)); + callback(f, node.model.c_str(), node.deviceId, node.nickName.c_str(), + node.serialNumber.c_str(), node.firmware.c_str()); } } @@ -333,14 +333,14 @@ extern "C" void set_dc_nickname(struct dive *dive) // we don't have this one, yet auto it = std::find_if(device_table.devices.begin(), device_table.devices.end(), [dc] (const device &dev) - { return !strcasecmp(qPrintable(dev.model), dc->model); }); + { return !strcasecmp(dev.model.c_str(), dc->model); }); if (it != device_table.devices.end()) { // we already have this model but a different deviceid - QString simpleNick(dc->model); + std::string simpleNick(dc->model); if (dc->deviceid == 0) - simpleNick.append(" (unknown deviceid)"); + simpleNick += " (unknown deviceid)"; else - simpleNick.append(" (").append(QString::number(dc->deviceid, 16)).append(")"); + simpleNick += " (" + QString::number(dc->deviceid, 16).toStdString() + ")"; addDC(device_table.devices, dc->model, dc->deviceid, simpleNick, {}, {}); } else { addDC(device_table.devices, dc->model, dc->deviceid, {}, {}, {}); @@ -349,12 +349,12 @@ extern "C" void set_dc_nickname(struct dive *dive) } } -QString get_dc_nickname(const struct divecomputer *dc) +const char *get_dc_nickname(const struct divecomputer *dc) { const device *existNode = getDCExact(device_table.devices, dc); - if (existNode && !existNode->nickName.isEmpty()) - return existNode->nickName; + if (existNode && !existNode->nickName.empty()) + return existNode->nickName.c_str(); else return dc->model; } diff --git a/core/device.h b/core/device.h index 910c77b90..90d67bb5e 100644 --- a/core/device.h +++ b/core/device.h @@ -16,6 +16,7 @@ extern void create_device_node(const char *model, uint32_t deviceid, const char extern void call_for_each_dc(void *f, void (*callback)(void *, const char *, uint32_t, const char *, const char *, const char *), bool select_only); extern void clear_device_nodes(); +const char *get_dc_nickname(const struct divecomputer *dc); #ifdef __cplusplus } @@ -24,18 +25,18 @@ extern void clear_device_nodes(); // Functions and global variables that are only available to C++ code #ifdef __cplusplus -#include +#include #include struct device { bool operator==(const device &a) const; bool operator!=(const device &a) const; bool operator<(const device &a) const; - void showchanges(const QString &n, const QString &s, const QString &f) const; - QString model; + void showchanges(const std::string &n, const std::string &s, const std::string &f) const; + std::string model; uint32_t deviceId; - QString serialNumber; - QString firmware; - QString nickName; + std::string serialNumber; + std::string firmware; + std::string nickName; }; struct device_table { @@ -43,7 +44,6 @@ struct device_table { QVector devices; }; -QString get_dc_nickname(const struct divecomputer *dc); extern struct device_table device_table; #endif diff --git a/qt-models/divecomputermodel.cpp b/qt-models/divecomputermodel.cpp index dcac0a0dc..c675ad641 100644 --- a/qt-models/divecomputermodel.cpp +++ b/qt-models/divecomputermodel.cpp @@ -20,9 +20,9 @@ QVariant DiveComputerModel::data(const QModelIndex &index, int role) const case ID: return QString("0x").append(QString::number(node.deviceId, 16)); case MODEL: - return node.model; + return QString::fromStdString(node.model); case NICKNAME: - return node.nickName; + return QString::fromStdString(node.nickName); } } @@ -59,7 +59,7 @@ bool DiveComputerModel::setData(const QModelIndex &index, const QVariant &value, return false; device &node = dcs[index.row()]; - node.nickName = value.toString(); + node.nickName = value.toString().toStdString(); emit dataChanged(index, index); return true; } -- cgit v1.2.3-70-g09d2