diff --git a/core/dive.c b/core/dive.c index 1a26ac5d7..811c0270c 100644 --- a/core/dive.c +++ b/core/dive.c @@ -12,6 +12,7 @@ #include "divelist.h" #include "qthelper.h" #include "metadata.h" +#include "membuffer.h" /* one could argue about the best place to have this variable - * it's used in the UI, but it seems to make the most sense to have it @@ -3020,30 +3021,28 @@ void taglist_cleanup(struct tag_entry **tag_list) } } -int taglist_get_tagstring(struct tag_entry *tag_list, char *buffer, int len) +char *taglist_get_tagstring(struct tag_entry *tag_list) { - int i = 0; - struct tag_entry *tmp; - tmp = tag_list; - memset(buffer, 0, len); + bool first_tag = true; + struct membuffer b = { 0 }; + struct tag_entry *tmp = tag_list; while (tmp != NULL) { - int newlength = strlen(tmp->tag->name); - if (i > 0) - newlength += 2; - if ((i + newlength) < len) { - if (i > 0) { - strcpy(buffer + i, ", "); - strcpy(buffer + i + 2, tmp->tag->name); + if (!empty_string(tmp->tag->name)) { + if (first_tag) { + put_format(&b, "%s", tmp->tag->name); + first_tag = false; } else { - strcpy(buffer, tmp->tag->name); + put_format(&b, ", %s", tmp->tag->name); } - } else { - return i; } - i += newlength; tmp = tmp->next; } - return i; + /* Ensures we do return null terminated empty string for: + * - empty tag list + * - tag list with empty tag only + */ + mb_cstring(&b); + return detach_buffer(&b); } static inline void taglist_free_divetag(struct divetag *tag) diff --git a/core/dive.h b/core/dive.h index 62fe31917..ca0325192 100644 --- a/core/dive.h +++ b/core/dive.h @@ -264,10 +264,13 @@ struct tag_entry *taglist_added(struct tag_entry *original_list, struct tag_entr void dump_taglist(const char *intro, struct tag_entry *tl); /* - * Writes all divetags in tag_list to buffer, limited by the buffer's (len)gth. - * Returns the characters written + * Writes all divetags form tag_list into internally allocated buffer + * Function returns pointer to allocated buffer + * Buffer contains comma separated list of tags names or null terminated string + * + * NOTE! The returned buffer must be freed once used. */ -int taglist_get_tagstring(struct tag_entry *tag_list, char *buffer, int len); +char *taglist_get_tagstring(struct tag_entry *tag_list); /* cleans up a list: removes empty tags and duplicates */ void taglist_cleanup(struct tag_entry **tag_list); diff --git a/core/qthelper.cpp b/core/qthelper.cpp index 1cef38184..d63ee3568 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -1282,6 +1282,14 @@ QString get_divepoint_gas_string(struct dive *d, const divedatapoint &p) return get_gas_string(d->cylinder[idx].gasmix); } +QString get_taglist_string(struct tag_entry *tag_list) +{ + char *buffer = taglist_get_tagstring(tag_list); + QString ret = QString::fromUtf8(buffer); + free(buffer); + return ret; +} + weight_t string_to_weight(const char *str) { const char *end; diff --git a/core/qthelper.h b/core/qthelper.h index 978493e1c..c699c5653 100644 --- a/core/qthelper.h +++ b/core/qthelper.h @@ -30,6 +30,7 @@ bool gpsHasChanged(struct dive *dive, struct dive *master, const QString &gps_te QList getDivesInTrip(dive_trip_t *trip); QString get_gas_string(struct gasmix gas); QString get_divepoint_gas_string(struct dive *d, const divedatapoint& dp); +QString get_taglist_string(struct tag_entry *tag_list); void read_hashes(); void write_hashes(); void updateHash(struct picture *picture); diff --git a/core/subsurface-qt/DiveObjectHelper.cpp b/core/subsurface-qt/DiveObjectHelper.cpp index 093451ad3..4c1c66e21 100644 --- a/core/subsurface-qt/DiveObjectHelper.cpp +++ b/core/subsurface-qt/DiveObjectHelper.cpp @@ -192,9 +192,7 @@ QString DiveObjectHelper::notes() const QString DiveObjectHelper::tags() const { - static char buffer[256]; - taglist_get_tagstring(m_dive->tag_list, buffer, 256); - return QString(buffer); + return get_taglist_string(m_dive->tag_list); } QString DiveObjectHelper::gas() const diff --git a/desktop-widgets/tab-widgets/maintab.cpp b/desktop-widgets/tab-widgets/maintab.cpp index 82e9b8abc..4f80454e8 100644 --- a/desktop-widgets/tab-widgets/maintab.cpp +++ b/desktop-widgets/tab-widgets/maintab.cpp @@ -417,7 +417,6 @@ void MainTab::updateDiveInfo(bool clear) // for the trip in the Info tab, otherwise we show the info of the // selected_dive struct dive *prevd; - char buf[1024]; process_selected_dives(); process_all_dives(&displayed_dive, &prevd); @@ -570,8 +569,7 @@ void MainTab::updateDiveInfo(bool clear) ui.equipmentTab->setEnabled(true); cylindersModel->updateDive(); weightModel->updateDive(); - taglist_get_tagstring(displayed_dive.tag_list, buf, 1024); - ui.tagWidget->setText(QString(buf)); + ui.tagWidget->setText(get_taglist_string(displayed_dive.tag_list)); if (current_dive) { bool isManual = same_string(current_dive->dc.model, "manually added dive"); ui.depth->setVisible(isManual); @@ -1347,13 +1345,10 @@ void MainTab::diffTaggedStrings(QString currentString, QString displayedString, void MainTab::on_tagWidget_textChanged() { - char buf[1024]; - if (editMode == IGNORE || acceptingEdit == true) return; - taglist_get_tagstring(displayed_dive.tag_list, buf, 1024); - if (same_string(buf, qPrintable(ui.tagWidget->toPlainText()))) + if (get_taglist_string(displayed_dive.tag_list) == ui.tagWidget->toPlainText()) return; markChangedWidget(ui.tagWidget); @@ -1524,9 +1519,7 @@ void MainTab::showAndTriggerEditSelective(struct dive_components what) if (what.divesite) ui.location->setCurrentDiveSiteUuid(displayed_dive.dive_site_uuid); if (what.tags) { - char buf[1024]; - taglist_get_tagstring(displayed_dive.tag_list, buf, 1024); - ui.tagWidget->setText(QString(buf)); + ui.tagWidget->setText(get_taglist_string(displayed_dive.tag_list)); } if (what.cylinders) { cylindersModel->updateDive(); diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index 082d3212e..7df52c94a 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -422,12 +422,7 @@ QString DiveItem::displayWeightWithUnit() const QString DiveItem::displayTags() const { struct dive *dive = get_dive_by_uniq_id(diveId); - if (!dive->tag_list) - return QString(); - - char buf[1024]; - taglist_get_tagstring(dive->tag_list, buf, 1024); - return QString(buf); + return get_taglist_string(dive->tag_list); } int DiveItem::weight() const diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2efacd706..a9c65bd09 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -87,6 +87,7 @@ TEST(TestGitStorage testgitstorage.cpp) TEST(TestPreferences testpreferences.cpp) TEST(TestPicture testpicture.cpp) TEST(TestMerge testmerge.cpp) +TEST(TestTagList testtaglist.cpp) add_custom_target(check COMMAND ${CMAKE_CTEST_COMMAND} @@ -102,6 +103,7 @@ add_custom_target(check COMMAND ${CMAKE_CTEST_COMMAND} TestRenumber TestPicture TestMerge + TestTagList ) # useful for debugging CMake issues diff --git a/tests/testtaglist.cpp b/tests/testtaglist.cpp new file mode 100644 index 000000000..afe5049f1 --- /dev/null +++ b/tests/testtaglist.cpp @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "testtaglist.h" +#include "core/dive.h" + +void TestTagList::initTestCase() +{ + taglist_init_global(); +} + +void TestTagList::cleanupTestCase() +{ + taglist_free(g_tag_list); + g_tag_list = NULL; +} + +void TestTagList::testGetTagstringNoTags() +{ + struct tag_entry *tag_list = NULL; + char *tagstring = taglist_get_tagstring(tag_list); + QVERIFY(tagstring != NULL); + QCOMPARE(*tagstring, '\0'); +} + +void TestTagList::testGetTagstringSingleTag() +{ + struct tag_entry *tag_list = NULL; + taglist_add_tag(&tag_list, "A new tag"); + char *tagstring = taglist_get_tagstring(tag_list); + QVERIFY(tagstring != NULL); + QCOMPARE(QString::fromUtf8(tagstring), QString::fromUtf8("A new tag")); + free(tagstring); +} + +void TestTagList::testGetTagstringMultipleTags() +{ + struct tag_entry *tag_list = NULL; + taglist_add_tag(&tag_list, "A new tag"); + taglist_add_tag(&tag_list, "A new tag 1"); + taglist_add_tag(&tag_list, "A new tag 2"); + taglist_add_tag(&tag_list, "A new tag 3"); + taglist_add_tag(&tag_list, "A new tag 4"); + taglist_add_tag(&tag_list, "A new tag 5"); + char *tagstring = taglist_get_tagstring(tag_list); + QVERIFY(tagstring != NULL); + QCOMPARE(QString::fromUtf8(tagstring), + QString::fromUtf8( + "A new tag, " + "A new tag 1, " + "A new tag 2, " + "A new tag 3, " + "A new tag 4, " + "A new tag 5")); + free(tagstring); +} + +void TestTagList::testGetTagstringWithAnEmptyTag() +{ + struct tag_entry *tag_list = NULL; + taglist_add_tag(&tag_list, "A new tag"); + taglist_add_tag(&tag_list, "A new tag 1"); + taglist_add_tag(&tag_list, ""); + char *tagstring = taglist_get_tagstring(tag_list); + QVERIFY(tagstring != NULL); + QCOMPARE(QString::fromUtf8(tagstring), + QString::fromUtf8( + "A new tag, " + "A new tag 1")); + free(tagstring); +} + +void TestTagList::testGetTagstringEmptyTagOnly() +{ + struct tag_entry *tag_list = NULL; + taglist_add_tag(&tag_list, ""); + char *tagstring = taglist_get_tagstring(tag_list); + QVERIFY(tagstring != NULL); + QCOMPARE(QString::fromUtf8(tagstring), + QString::fromUtf8("")); + free(tagstring); +} + +QTEST_GUILESS_MAIN(TestTagList) diff --git a/tests/testtaglist.h b/tests/testtaglist.h new file mode 100644 index 000000000..7e6e94742 --- /dev/null +++ b/tests/testtaglist.h @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 +#ifndef TESTTAGLIST_H +#define TESTTAGLIST_H + +#include + +class TestTagList : public QObject { + Q_OBJECT +private slots: + void initTestCase(); + void cleanupTestCase(); + + void testGetTagstringNoTags(); + void testGetTagstringSingleTag(); + void testGetTagstringMultipleTags(); + void testGetTagstringWithAnEmptyTag(); + void testGetTagstringEmptyTagOnly(); +}; + +#endif