mifr/subsurface
1
0
Fork 0
mirror of https://github.com/subsurface/subsurface.git synced 2024-11-30 22:20:21 +00:00
Code Issues Projects Releases Packages Wiki Activity

cleanup: use std::string in struct device

Browse source 
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 <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2020-10-05 09:56:21 +02:00 committed by Dirk Hohndel
parent 4e479677a0
commit fd8bd9d5c7
3 changed files with 37 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

54
core/device.cpp
View file

@ -2,11 +2,11 @@
#include "ssrf.h" #include "ssrf.h"
#include "dive.h" #include "dive.h"
#include "subsurface-string.h" #include "subsurface-string.h"
#include "qthelper.h" // for copy_qstring
#include "device.h" #include "device.h"
#include "errorhelper.h" // for verbose flag #include "errorhelper.h" // for verbose flag
#include "selection.h" #include "selection.h"
#include "core/settings/qPrefDiveComputer.h" #include "core/settings/qPrefDiveComputer.h"
#include <QString> // for QString::number
/* /*
* Good fake dive profiles are hard. * Good fake dive profiles are hard.
@ -238,29 +238,29 @@ extern "C" void set_dc_deviceid(struct divecomputer *dc, unsigned int deviceid)
if (!node) if (!node)
return; return;
if (!node->serialNumber.isEmpty() && empty_string(dc->serial)) { if (!node->serialNumber.empty() && empty_string(dc->serial)) {
free((void *)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); 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()) if (nickName != n && !n.empty())
qDebug("new nickname %s for DC model %s deviceId 0x%x", qPrintable(n), qPrintable(model), deviceId); qDebug("new nickname %s for DC model %s deviceId 0x%x", n.c_str(), model.c_str(), deviceId);
if (serialNumber != s && !s.isEmpty()) if (serialNumber != s && !s.empty())
qDebug("new serial number %s for DC model %s deviceId 0x%x", qPrintable(s), qPrintable(model), deviceId); qDebug("new serial number %s for DC model %s deviceId 0x%x", s.c_str(), model.c_str(), deviceId);
if (firmware != f && !f.isEmpty()) if (firmware != f && !f.empty())
qDebug("new firmware version %s for DC model %s deviceId 0x%x", qPrintable(f), qPrintable(model), deviceId); qDebug("new firmware version %s for DC model %s deviceId 0x%x", f.c_str(), model.c_str(), deviceId);
} }
static void addDC(QVector<device> &dcs, const QString &m, uint32_t d, const QString &n, const QString &s, const QString &f) static void addDC(QVector<device> &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; return;
auto it = std::lower_bound(dcs.begin(), dcs.end(), device{m, d, {}, {}, {}}); auto it = std::lower_bound(dcs.begin(), dcs.end(), device{m, d, {}, {}, {}});
if (it != dcs.end() && it->model == m && it->deviceId == d) { if (it != dcs.end() && it->model == m && it->deviceId == d) {
@ -268,11 +268,11 @@ static void addDC(QVector<device> &dcs, const QString &m, uint32_t d, const QStr
if (verbose) if (verbose)
it->showchanges(n, s, f); it->showchanges(n, s, f);
// Update any non-existent fields from the old entry // Update any non-existent fields from the old entry
if (!n.isEmpty()) if (!n.empty())
it->nickName = n; it->nickName = n;
if (!s.isEmpty()) if (!s.empty())
it->serialNumber = s; it->serialNumber = s;
if (!f.isEmpty()) if (!f.empty())
it->firmware = f; it->firmware = f;
} else { } else {
dcs.insert(it, device{m, d, s, f, n}); dcs.insert(it, device{m, d, s, f, n});
@ -281,7 +281,7 @@ static void addDC(QVector<device> &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) 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() 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; found = true;
} }
if (found) if (found)
callback(f, qPrintable(node.model), node.deviceId, qPrintable(node.nickName), callback(f, node.model.c_str(), node.deviceId, node.nickName.c_str(),
qPrintable(node.serialNumber), qPrintable(node.firmware)); 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 // we don't have this one, yet
auto it = std::find_if(device_table.devices.begin(), device_table.devices.end(), auto it = std::find_if(device_table.devices.begin(), device_table.devices.end(),
[dc] (const device &dev) [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()) { if (it != device_table.devices.end()) {
// we already have this model but a different deviceid // we already have this model but a different deviceid
QString simpleNick(dc->model); std::string simpleNick(dc->model);
if (dc->deviceid == 0) if (dc->deviceid == 0)
simpleNick.append(" (unknown deviceid)"); simpleNick += " (unknown deviceid)";
else 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, {}, {}); addDC(device_table.devices, dc->model, dc->deviceid, simpleNick, {}, {});
} else { } else {
addDC(device_table.devices, dc->model, dc->deviceid, {}, {}, {}); 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); const device *existNode = getDCExact(device_table.devices, dc);
if (existNode && !existNode->nickName.isEmpty()) if (existNode && !existNode->nickName.empty())
return existNode->nickName; return existNode->nickName.c_str();
else else
return dc->model; return dc->model;
} }

14
core/device.h
View file

@ -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, extern void call_for_each_dc(void *f, void (*callback)(void *, const char *, uint32_t,
const char *, const char *, const char *), bool select_only); const char *, const char *, const char *), bool select_only);
extern void clear_device_nodes(); extern void clear_device_nodes();
const char *get_dc_nickname(const struct divecomputer *dc);
#ifdef __cplusplus #ifdef __cplusplus
} }
@ -24,18 +25,18 @@ extern void clear_device_nodes();
// Functions and global variables that are only available to C++ code // Functions and global variables that are only available to C++ code
#ifdef __cplusplus #ifdef __cplusplus
#include <QString> #include <string>
#include <QVector> #include <QVector>
struct device { struct device {
bool operator==(const device &a) const; bool operator==(const device &a) const;
bool operator!=(const device &a) const; 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; void showchanges(const std::string &n, const std::string &s, const std::string &f) const;
QString model; std::string model;
uint32_t deviceId; uint32_t deviceId;
QString serialNumber; std::string serialNumber;
QString firmware; std::string firmware;
QString nickName; std::string nickName;
}; };
struct device_table { struct device_table {
@ -43,7 +44,6 @@ struct device_table {
QVector<device> devices; QVector<device> devices;
}; };
QString get_dc_nickname(const struct divecomputer *dc);
extern struct device_table device_table; extern struct device_table device_table;
#endif #endif

6
qt-models/divecomputermodel.cpp
View file

@ -20,9 +20,9 @@ QVariant DiveComputerModel::data(const QModelIndex &index, int role) const
case ID: case ID:
return QString("0x").append(QString::number(node.deviceId, 16)); return QString("0x").append(QString::number(node.deviceId, 16));
case MODEL: case MODEL:
return node.model; return QString::fromStdString(node.model);
case NICKNAME: 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; return false;
device &node = dcs[index.row()]; device &node = dcs[index.row()];
node.nickName = value.toString(); node.nickName = value.toString().toStdString();
emit dataChanged(index, index); emit dataChanged(index, index);
return true; return true;
} }