From 50b11024d685129e78c36313b892dbc0e55c654c Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 11 Dec 2020 22:34:35 +0100 Subject: [PATCH] core: keep tank infos in a dynamic table The list of known tank types were kept in a fixed size table. Instead, use a dynamic table with our horrendous table macros. This is more flexible and sensible. While doing this, clean up the TankInfoModel, which was leaking memory. Signed-off-by: Berthold Stoeger --- commands/command_edit.cpp | 6 +- core/datatrak.c | 14 +- core/divelist.c | 2 + core/equipment.c | 166 ++++++++++-------- core/equipment.h | 15 +- core/subsurface-qt/diveobjecthelper.cpp | 4 +- .../preferences/preferences_equipment.cpp | 7 +- mobile-widgets/qmlmanager.cpp | 20 +-- qt-models/tankinfomodel.cpp | 55 +++--- qt-models/tankinfomodel.h | 4 - subsurface-desktop-main.cpp | 1 + subsurface-mobile-main.cpp | 1 + 12 files changed, 154 insertions(+), 141 deletions(-) diff --git a/commands/command_edit.cpp b/commands/command_edit.cpp index 686b5b91d..49b1c7cce 100644 --- a/commands/command_edit.cpp +++ b/commands/command_edit.cpp @@ -1207,9 +1207,9 @@ EditCylinder::EditCylinder(int index, cylinder_t cylIn, EditCylinderType typeIn, // Try to untranslate the cylinder type QString description = cylIn.type.description; - for (int i = 0; i < MAX_TANK_INFO && tank_info[i].name; ++i) { - if (gettextFromC::tr(tank_info[i].name) == description) { - description = tank_info[i].name; + for (int i = 0; i < tank_info_table.nr; ++i) { + if (gettextFromC::tr(tank_info_table.infos[i].name) == description) { + description = tank_info_table.infos[i].name; break; } } diff --git a/core/datatrak.c b/core/datatrak.c index e1a62f582..2fa89010e 100644 --- a/core/datatrak.c +++ b/core/datatrak.c @@ -130,14 +130,12 @@ static int dtrak_prepare_data(int model, device_data_t *dev_data) */ static const char *cyl_type_by_size(int size) { - struct tank_info_t *ti = tank_info; - - while (ti->ml != size && ti < tank_info + MAX_TANK_INFO) - ti++; - if (ti == tank_info + MAX_TANK_INFO) - return ""; - else - return ti->name; + for (int i = 0; i < tank_info_table.nr; ++i) { + const struct tank_info *ti = &tank_info_table.infos[i]; + if (ti->ml == size) + return ti->name; + } + return ""; } /* diff --git a/core/divelist.c b/core/divelist.c index 8552c2689..a15c40d83 100644 --- a/core/divelist.c +++ b/core/divelist.c @@ -1395,6 +1395,8 @@ void clear_dive_file_data() reset_min_datafile_version(); clear_git_id(); + reset_tank_info_table(&tank_info_table); + /* Inform frontend of reset data. This should reset all the models. */ emit_reset_signal(); } diff --git a/core/equipment.c b/core/equipment.c index 2fab787eb..bdecbb3f7 100644 --- a/core/equipment.c +++ b/core/equipment.c @@ -51,6 +51,11 @@ void copy_cylinders(const struct cylinder_table *s, struct cylinder_table *d) add_cloned_cylinder(d, s->cylinders[i]); } +static void free_tank_info(struct tank_info info) +{ + free((void *)info.name); +} + /* weightsystem table functions */ //static MAKE_GET_IDX(weightsystem_table, weightsystem_t, weightsystems) static MAKE_GROW_TABLE(weightsystem_table, weightsystem_t, weightsystems) @@ -71,6 +76,12 @@ static MAKE_REMOVE_FROM(cylinder_table, cylinders) //MAKE_REMOVE(cylinder_table, cylinder_t, cylinder) MAKE_CLEAR_TABLE(cylinder_table, cylinders, cylinder) +/* tank_info table functions */ +static MAKE_GROW_TABLE(tank_info_table, tank_info_t, infos) +static MAKE_ADD_TO(tank_info_table, tank_info_t, infos) +//static MAKE_REMOVE_FROM(tank_info_table, infos) +MAKE_CLEAR_TABLE(tank_info_table, infos, tank_info) + const char *cylinderuse_text[NUM_GAS_USE] = { QT_TRANSLATE_NOOP("gettextFromC", "OC-gas"), QT_TRANSLATE_NOOP("gettextFromC", "diluent"), QT_TRANSLATE_NOOP("gettextFromC", "oxygen"), QT_TRANSLATE_NOOP("gettextFromC", "not used") }; @@ -84,25 +95,32 @@ int cylinderuse_from_text(const char *text) return -1; } +/* Add a metric or an imperial tank info structure. Copies the passed-in string. */ +void add_tank_info_metric(struct tank_info_table *table, const char *name, int ml, int bar) +{ + struct tank_info info = { strdup(name), .ml = ml, .bar = bar }; + add_to_tank_info_table(table, table->nr, info); +} + +void add_tank_info_imperial(struct tank_info_table *table, const char *name, int cuft, int psi) +{ + struct tank_info info = { strdup(name), .cuft = cuft, .psi = psi }; + add_to_tank_info_table(table, table->nr, info); +} + /* placeholders for a few functions that we need to redesign for the Qt UI */ void add_cylinder_description(const cylinder_type_t *type) { - const char *desc; - int i; - - desc = type->description; - if (!desc) + const char *desc = type->description; + if (empty_string(desc)) return; - for (i = 0; i < MAX_TANK_INFO && tank_info[i].name != NULL; i++) { - if (strcmp(tank_info[i].name, desc) == 0) + for (int i = 0; i < tank_info_table.nr; i++) { + if (strcmp(tank_info_table.infos[i].name, desc) == 0) return; } - if (i < MAX_TANK_INFO) { - // FIXME: leaked on exit - tank_info[i].name = strdup(desc); - tank_info[i].ml = type->size.mliter; - tank_info[i].bar = type->workingpressure.mbar / 1000; - } + // FIXME: leaked on exit + add_tank_info_metric(&tank_info_table, desc, type->size.mliter, + type->workingpressure.mbar / 1000); } void add_weightsystem_description(const weightsystem_t *weightsystem) @@ -229,62 +247,63 @@ int find_best_gasmix_match(struct gasmix mix, const struct cylinder_table *cylin * we should pick up any other names from the dive * logs directly. */ -struct tank_info_t tank_info[MAX_TANK_INFO] = { +struct tank_info_table tank_info_table; +void reset_tank_info_table(struct tank_info_table *table) +{ + clear_tank_info_table(table); + /* Need an empty entry for the no-cylinder case */ - { "", }, + add_tank_info_metric(table, "", 0, 0); /* Size-only metric cylinders */ - { "10.0ℓ", .ml = 10000 }, - { "11.1ℓ", .ml = 11100 }, + add_tank_info_metric(table, "10.0ℓ", 10000, 0); + add_tank_info_metric(table, "11.1ℓ", 11100, 0); /* Most common AL cylinders */ - { "AL40", .cuft = 40, .psi = 3000 }, - { "AL50", .cuft = 50, .psi = 3000 }, - { "AL63", .cuft = 63, .psi = 3000 }, - { "AL72", .cuft = 72, .psi = 3000 }, - { "AL80", .cuft = 80, .psi = 3000 }, - { "AL100", .cuft = 100, .psi = 3300 }, + add_tank_info_metric(table, "AL40", 40, 3000); + add_tank_info_metric(table, "AL50", 50, 3000); + add_tank_info_metric(table, "AL63", 63, 3000); + add_tank_info_metric(table, "AL72", 72, 3000); + add_tank_info_metric(table, "AL80", 80, 3000); + add_tank_info_metric(table, "AL100", 100, 3300); /* Metric AL cylinders */ - { "ALU7", .ml = 7000, .bar = 200 }, + add_tank_info_metric(table, "ALU7", 7000, 200); /* Somewhat common LP steel cylinders */ - { "LP85", .cuft = 85, .psi = 2640 }, - { "LP95", .cuft = 95, .psi = 2640 }, - { "LP108", .cuft = 108, .psi = 2640 }, - { "LP121", .cuft = 121, .psi = 2640 }, + add_tank_info_imperial(table, "LP85", 85, 2640); + add_tank_info_imperial(table, "LP95", 95, 2640); + add_tank_info_imperial(table, "LP108", 108, 2640); + add_tank_info_imperial(table, "LP121", 121, 2640); /* Somewhat common HP steel cylinders */ - { "HP65", .cuft = 65, .psi = 3442 }, - { "HP80", .cuft = 80, .psi = 3442 }, - { "HP100", .cuft = 100, .psi = 3442 }, - { "HP117", .cuft = 117, .psi = 3442 }, - { "HP119", .cuft = 119, .psi = 3442 }, - { "HP130", .cuft = 130, .psi = 3442 }, + add_tank_info_imperial(table, "HP65", 65, 3442); + add_tank_info_imperial(table, "HP80", 80, 3442); + add_tank_info_imperial(table, "HP100", 100, 3442); + add_tank_info_imperial(table, "HP117", 117, 3442); + add_tank_info_imperial(table, "HP119", 119, 3442); + add_tank_info_imperial(table, "HP130", 130, 3442); /* Common European steel cylinders */ - { "3ℓ 232 bar", .ml = 3000, .bar = 232 }, - { "3ℓ 300 bar", .ml = 3000, .bar = 300 }, - { "10ℓ 200 bar", .ml = 10000, .bar = 200 }, - { "10ℓ 232 bar", .ml = 10000, .bar = 232 }, - { "10ℓ 300 bar", .ml = 10000, .bar = 300 }, - { "12ℓ 200 bar", .ml = 12000, .bar = 200 }, - { "12ℓ 232 bar", .ml = 12000, .bar = 232 }, - { "12ℓ 300 bar", .ml = 12000, .bar = 300 }, - { "15ℓ 200 bar", .ml = 15000, .bar = 200 }, - { "15ℓ 232 bar", .ml = 15000, .bar = 232 }, - { "D7 300 bar", .ml = 14000, .bar = 300 }, - { "D8.5 232 bar", .ml = 17000, .bar = 232 }, - { "D12 232 bar", .ml = 24000, .bar = 232 }, - { "D13 232 bar", .ml = 26000, .bar = 232 }, - { "D15 232 bar", .ml = 30000, .bar = 232 }, - { "D16 232 bar", .ml = 32000, .bar = 232 }, - { "D18 232 bar", .ml = 36000, .bar = 232 }, - { "D20 232 bar", .ml = 40000, .bar = 232 }, - - /* We'll fill in more from the dive log dynamically */ - { NULL, } -}; + add_tank_info_metric(table, "3ℓ 232 bar", 3000, 232); + add_tank_info_metric(table, "3ℓ 300 bar", 3000, 300); + add_tank_info_metric(table, "10ℓ 200 bar", 10000, 200); + add_tank_info_metric(table, "10ℓ 232 bar", 10000, 232); + add_tank_info_metric(table, "10ℓ 300 bar", 10000, 300); + add_tank_info_metric(table, "12ℓ 200 bar", 12000, 200); + add_tank_info_metric(table, "12ℓ 232 bar", 12000, 232); + add_tank_info_metric(table, "12ℓ 300 bar", 12000, 300); + add_tank_info_metric(table, "15ℓ 200 bar", 15000, 200); + add_tank_info_metric(table, "15ℓ 232 bar", 15000, 232); + add_tank_info_metric(table, "D7 300 bar", 14000, 300); + add_tank_info_metric(table, "D8.5 232 bar", 17000, 232); + add_tank_info_metric(table, "D12 232 bar", 24000, 232); + add_tank_info_metric(table, "D13 232 bar", 26000, 232); + add_tank_info_metric(table, "D15 232 bar", 30000, 232); + add_tank_info_metric(table, "D16 232 bar", 32000, 232); + add_tank_info_metric(table, "D18 232 bar", 36000, 232); + add_tank_info_metric(table, "D20 232 bar", 40000, 232); +} /* * We hardcode the most common weight system types @@ -402,30 +421,27 @@ cylinder_t *get_or_create_cylinder(struct dive *d, int idx) void fill_default_cylinder(const struct dive *dive, cylinder_t *cyl) { const char *cyl_name = prefs.default_cylinder; - struct tank_info_t *ti = tank_info; pressure_t pO2 = {.mbar = lrint(prefs.modpO2 * 1000.0)}; if (!cyl_name) return; - while (ti->name != NULL && ti < tank_info + MAX_TANK_INFO) { - if (strcmp(ti->name, cyl_name) == 0) - break; - ti++; + for (int i = 0; i < tank_info_table.nr; ++i) { + struct tank_info *ti = &tank_info_table.infos[i]; + if (strcmp(ti->name, cyl_name) == 0) { + cyl->type.description = strdup(ti->name); + if (ti->ml) { + cyl->type.size.mliter = ti->ml; + cyl->type.workingpressure.mbar = ti->bar * 1000; + } else { + cyl->type.workingpressure.mbar = psi_to_mbar(ti->psi); + if (ti->psi) + cyl->type.size.mliter = lrint(cuft_to_l(ti->cuft) * 1000 / bar_to_atm(psi_to_bar(ti->psi))); + } + // MOD of air + cyl->depth = gas_mod(cyl->gasmix, pO2, dive, 1); + return; + } } - if (ti->name == NULL) - /* didn't find it */ - return; - cyl->type.description = strdup(ti->name); - if (ti->ml) { - cyl->type.size.mliter = ti->ml; - cyl->type.workingpressure.mbar = ti->bar * 1000; - } else { - cyl->type.workingpressure.mbar = psi_to_mbar(ti->psi); - if (ti->psi) - cyl->type.size.mliter = lrint(cuft_to_l(ti->cuft) * 1000 / bar_to_atm(psi_to_bar(ti->psi))); - } - // MOD of air - cyl->depth = gas_mod(cyl->gasmix, pO2, dive, 1); } cylinder_t create_new_cylinder(const struct dive *d) diff --git a/core/equipment.h b/core/equipment.h index 96cc43f1f..8f7cee74a 100644 --- a/core/equipment.h +++ b/core/equipment.h @@ -68,7 +68,6 @@ struct weightsystem_table { weightsystem_t *weightsystems; }; -#define MAX_TANK_INFO (100) #define MAX_WS_INFO (100) extern int cylinderuse_from_text(const char *text); @@ -110,11 +109,21 @@ extern void add_cylinder(struct cylinder_table *, int idx, cylinder_t cyl); void get_gas_string(struct gasmix gasmix, char *text, int len); const char *gasname(struct gasmix gasmix); -struct tank_info_t { +typedef struct tank_info { const char *name; int cuft, ml, psi, bar; +} tank_info_t; + +struct tank_info_table { + int nr, allocated; + struct tank_info *infos; }; -extern struct tank_info_t tank_info[MAX_TANK_INFO]; + +extern struct tank_info_table tank_info_table; +extern void reset_tank_info_table(struct tank_info_table *table); +extern void clear_tank_info_table(struct tank_info_table *table); +extern void add_tank_info_metric(struct tank_info_table *table, const char *name, int ml, int bar); +extern void add_tank_info_imperial(struct tank_info_table *table, const char *name, int cuft, int psi); struct ws_info_t { const char *name; diff --git a/core/subsurface-qt/diveobjecthelper.cpp b/core/subsurface-qt/diveobjecthelper.cpp index 1df949b0f..1fa91a301 100644 --- a/core/subsurface-qt/diveobjecthelper.cpp +++ b/core/subsurface-qt/diveobjecthelper.cpp @@ -241,8 +241,8 @@ QStringList getFullCylinderList() addStringToSortedList(cylinders, get_cylinder(d, j)->type.description); } - for (int ti = 0; ti < MAX_TANK_INFO; ti++) - addStringToSortedList(cylinders, tank_info[ti].name); + for (int ti = 0; ti < tank_info_table.nr; ti++) + addStringToSortedList(cylinders, tank_info_table.infos[ti].name); return cylinders; } diff --git a/desktop-widgets/preferences/preferences_equipment.cpp b/desktop-widgets/preferences/preferences_equipment.cpp index 7435f3539..00bd9aa53 100644 --- a/desktop-widgets/preferences/preferences_equipment.cpp +++ b/desktop-widgets/preferences/preferences_equipment.cpp @@ -24,9 +24,10 @@ void PreferencesEquipment::refreshSettings() { ui->display_unused_tanks->setChecked(prefs.display_unused_tanks); ui->default_cylinder->clear(); - for (int i = 0; i < MAX_TANK_INFO && tank_info[i].name != NULL; i++) { - ui->default_cylinder->addItem(tank_info[i].name); - if (qPrefEquipment::default_cylinder() == tank_info[i].name) + for (int i = 0; i < tank_info_table.nr; i++) { + const tank_info &ti = tank_info_table.infos[i]; + ui->default_cylinder->addItem(ti.name); + if (qPrefEquipment::default_cylinder() == ti.name) ui->default_cylinder->setCurrentIndex(i); } } diff --git a/mobile-widgets/qmlmanager.cpp b/mobile-widgets/qmlmanager.cpp index 10f6f11e5..1b5c1ed17 100644 --- a/mobile-widgets/qmlmanager.cpp +++ b/mobile-widgets/qmlmanager.cpp @@ -1156,20 +1156,20 @@ void QMLManager::commitChanges(QString diveId, QString number, QString date, QSt // info for first cylinder if (myDive.getCylinder != usedCylinder) { diveChanged = true; - unsigned long i; int size = 0, wp = 0, j = 0, k = 0; for (j = 0; k < usedCylinder.length(); j++) { if (state != "add" && !is_cylinder_used(d, j)) continue; - for (i = 0; i < MAX_TANK_INFO && tank_info[i].name != NULL; i++) { - if (tank_info[i].name == usedCylinder[k] ) { - if (tank_info[i].ml > 0){ - size = tank_info[i].ml; - wp = tank_info[i].bar * 1000; + for (int i = 0; i < tank_info_table.nr; i++) { + const tank_info &ti = tank_info_table.infos[i]; + if (ti.name == usedCylinder[k] ) { + if (ti.ml > 0){ + size = ti.ml; + wp = ti.bar * 1000; } else { - size = (int) (cuft_to_l(tank_info[i].cuft) * 1000 / bar_to_atm(psi_to_bar(tank_info[i].psi))); - wp = psi_to_mbar(tank_info[i].psi); + size = (int) (cuft_to_l(ti.cuft) * 1000 / bar_to_atm(psi_to_bar(ti.psi))); + wp = psi_to_mbar(ti.psi); } break; } @@ -1816,8 +1816,8 @@ QStringList QMLManager::cylinderInit() const } } - for (unsigned long ti = 0; ti < MAX_TANK_INFO && tank_info[ti].name != NULL; ti++) { - QString cyl = tank_info[ti].name; + for (int ti = 0; ti < tank_info_table.nr; ti++) { + QString cyl = tank_info_table.infos[ti].name; if (cyl == "") continue; cylinders << cyl; diff --git a/qt-models/tankinfomodel.cpp b/qt-models/tankinfomodel.cpp index 76d921c71..0a9379ec9 100644 --- a/qt-models/tankinfomodel.cpp +++ b/qt-models/tankinfomodel.cpp @@ -13,8 +13,9 @@ TankInfoModel *TankInfoModel::instance() bool TankInfoModel::insertRows(int, int count, const QModelIndex &parent) { - beginInsertRows(parent, rowCount(), rowCount()); - rows += count; + beginInsertRows(parent, rowCount(), rowCount() + count - 1); + for (int i = 0; i < count; ++i) + add_tank_info_metric(&tank_info_table, "", 0, 0); endInsertRows(); return true; } @@ -23,64 +24,55 @@ bool TankInfoModel::setData(const QModelIndex &index, const QVariant &value, int { //WARN Seems wrong, we need to check for role == Qt::EditRole - if (index.row() < 0 || index.row() > MAX_TANK_INFO - 1) + if (index.row() < 0 || index.row() >= tank_info_table.nr ) return false; - struct tank_info_t *info = &tank_info[index.row()]; + struct tank_info &info = tank_info_table.infos[index.row()]; switch (index.column()) { case DESCRIPTION: - info->name = strdup(value.toByteArray().data()); + free((void *)info.name); + info.name = strdup(value.toByteArray().data()); break; case ML: - info->ml = value.toInt(); + info.ml = value.toInt(); break; case BAR: - info->bar = value.toInt(); + info.bar = value.toInt(); break; } emit dataChanged(index, index); return true; } -void TankInfoModel::clear() -{ -} - QVariant TankInfoModel::data(const QModelIndex &index, int role) const { - QVariant ret; - if (!index.isValid() || index.row() < 0 || index.row() > MAX_TANK_INFO - 1) { - return ret; - } - if (role == Qt::FontRole) { + if (!index.isValid() || index.row() < 0 || index.row() >= tank_info_table.nr) + return QVariant(); + if (role == Qt::FontRole) return defaultModelFont(); - } if (role == Qt::DisplayRole || role == Qt::EditRole) { - struct tank_info_t *info = &tank_info[index.row()]; - int ml = info->ml; - double bar = (info->psi) ? psi_to_bar(info->psi) : info->bar; + const struct tank_info &info = tank_info_table.infos[index.row()]; + int ml = info.ml; + double bar = (info.psi) ? psi_to_bar(info.psi) : info.bar; - if (info->cuft && info->psi) - ml = lrint(cuft_to_l(info->cuft) * 1000 / bar_to_atm(bar)); + if (info.cuft && info.psi) + ml = lrint(cuft_to_l(info.cuft) * 1000 / bar_to_atm(bar)); switch (index.column()) { case BAR: - ret = bar * 1000; - break; + return bar * 1000; case ML: - ret = ml; - break; + return ml; case DESCRIPTION: - ret = QString(info->name); - break; + return info.name; } } - return ret; + return QVariant(); } int TankInfoModel::rowCount(const QModelIndex&) const { - return rows; + return tank_info_table.nr; } TankInfoModel::TankInfoModel() @@ -94,8 +86,5 @@ TankInfoModel::TankInfoModel() void TankInfoModel::update() { beginResetModel(); - rows = 0; - for (struct tank_info_t *info = tank_info; info->name && info < tank_info + MAX_TANK_INFO; info++, rows++) - ; endResetModel(); } diff --git a/qt-models/tankinfomodel.h b/qt-models/tankinfomodel.h index 0317b054b..2fad322ea 100644 --- a/qt-models/tankinfomodel.h +++ b/qt-models/tankinfomodel.h @@ -22,13 +22,9 @@ public: int rowCount(const QModelIndex &parent = QModelIndex()) const override; bool insertRows(int row, int count, const QModelIndex &parent = QModelIndex()) override; bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::EditRole) override; - void clear(); public slots: void update(); - -private: - int rows; }; #endif diff --git a/subsurface-desktop-main.cpp b/subsurface-desktop-main.cpp index 6f662ecd7..af7352ec5 100644 --- a/subsurface-desktop-main.cpp +++ b/subsurface-desktop-main.cpp @@ -77,6 +77,7 @@ int main(int argc, char **argv) setup_system_prefs(); copy_prefs(&default_prefs, &prefs); fill_computer_list(); + reset_tank_info_table(&tank_info_table); parse_xml_init(); taglist_init_global(); init_ui(); diff --git a/subsurface-mobile-main.cpp b/subsurface-mobile-main.cpp index efaeeac36..255f20053 100644 --- a/subsurface-mobile-main.cpp +++ b/subsurface-mobile-main.cpp @@ -56,6 +56,7 @@ int main(int argc, char **argv) default_prefs.units = IMPERIAL_units; copy_prefs(&default_prefs, &prefs); fill_computer_list(); + reset_tank_info_table(&tank_info_table); parse_xml_init(); taglist_init_global();