refactor garmin_fs class (#1203)

* use container for garmin_fs ilinks. and leave memory management up to the copy on write container. * make gmsd const for read only usage. this prevents the possiblity of detachment of the ilinks list. * incorporate some variables and functions into garmin_fs_t * unify garmin category conversions. * fiddle with test time zone sensitivity. * add utc option for garmin_txt reader. * correct sign of adjustment * update garmin_txt includes.
GPSBabel · Nov 1, 2023 · bc0e8b9 · bc0e8b9
1 parent 1fcdf28
commit bc0e8b9
Show file tree

Hide file tree

Showing 16 changed files with 219 additions and 217 deletions.
diff --git a/garmin.cc b/garmin.cc
@@ -1241,7 +1241,7 @@ d103_icon_number_from_symbol(const QString& s)
 static void
 garmin_fs_garmin_after_read(const GPS_PWay way, Waypoint* wpt, const int protoid)
 {
-  garmin_fs_t* gmsd = garmin_fs_alloc(protoid);
+  auto* gmsd = new garmin_fs_t(protoid);
   wpt->fs.FsChainAdd(gmsd);
 
   /* nothing happens until gmsd is allocated some lines above */
@@ -1275,7 +1275,7 @@ garmin_fs_garmin_after_read(const GPS_PWay way, Waypoint* wpt, const int protoid
 static void
 garmin_fs_garmin_before_write(const Waypoint* wpt, GPS_PWay way, const int protoid)
 {
-  garmin_fs_t* gmsd = garmin_fs_t::find(wpt);
+  const garmin_fs_t* gmsd = garmin_fs_t::find(wpt);
 
   (void)protoid; // unused for now.
 

diff --git a/garmin_fs.cc b/garmin_fs.cc
@@ -29,92 +29,61 @@
 #include "inifile.h"  // for inifile_readstr
 
 
-#define MYNAME "garmin_fs"
-
-garmin_fs_t*
-garmin_fs_alloc(const int protocol)
-{
-  auto* result = new garmin_fs_t;
-
-  result->protocol = protocol;
-
-  return result;
-}
-
-garmin_fs_t* garmin_fs_t::clone() const
+std::optional<uint16_t>
+garmin_fs_t::convert_category(const QString& category_name)
 {
-  auto* copy = new garmin_fs_t(*this);
+  std::optional<uint16_t> category;
 
-  /* do not deep copy interlinks, only increment the reference counter */
-  if (ilinks != nullptr) {
-    ilinks->ref_count++;
-  }
-
-#ifdef GMSD_EXPERIMENTAL
-  memcopy(subclass, other.subclass, sizeof(subclass));
-#endif
-
-  return copy;
-}
-
-garmin_fs_t::~garmin_fs_t()
-{
-  garmin_ilink_t* links;
-  if ((links = ilinks) != nullptr) {
-    links->ref_count--;
-    if (links->ref_count <= 0) {
-      while (links != nullptr) {
-        garmin_ilink_t* tmp = links;
-        links = links->next;
-        xfree(tmp);
-      }
-    }
-  }
-}
-
-bool
-garmin_fs_convert_category(const QString& category_name, uint16_t* category)
-{
   // Is the name  "Category" followed by a number? Use that number.
   if (category_name.startsWith(u"Category ", Qt::CaseInsensitive)) {
     bool ok;
     int i = category_name.mid(9).toInt(&ok);
     if (ok && (i >= 1) && (i <= 16)) {
-      *category = (1 << --i);
-      return true;
+      category = (1 << --i);
+      return category;
     }
   }
   if (global_opts.inifile != nullptr) {
     // Do we have a gpsbabel.ini that maps category names to category #'s?
     for (int i = 0; i < 16; i++) {
       QString key = QString::number(i + 1);
-      QString c = inifile_readstr(global_opts.inifile, GMSD_SECTION_CATEGORIES, key);
+      QString c = inifile_readstr(global_opts.inifile, kGmsdSectionCategories, key);
       if (c.compare(category_name, Qt::CaseInsensitive) == 0) {
-        *category = (1 << i);
-        return true;
+        category = (1 << i);
+        return category;
       }
     }
   }
-  return false;
+  return category;
 }
 
-bool
-garmin_fs_merge_category(const QString& category_name, Waypoint* waypt)
+QStringList
+garmin_fs_t::print_categories(uint16_t categories)
 {
-  uint16_t cat;
+  QStringList categoryList;
 
-  // Attempt to get a textual category name to a category number.
-  if (!garmin_fs_convert_category(category_name, &cat)) {
-    return false;
+  if (categories == 0) {
+    return categoryList;
   }
 
-  garmin_fs_t* gmsd = garmin_fs_t::find(waypt);
-  cat = cat | (garmin_fs_t::get_category(gmsd, 0));
+  for (int i = 0; i < 16; i++) {
+    if ((categories & 1) != 0) {
+      QString c;
+      if (global_opts.inifile != nullptr) {
+        QString key = QString::number(i + 1);
+        c = inifile_readstr(global_opts.inifile, kGmsdSectionCategories, key);
+      }
 
-  if (gmsd == nullptr) {
-    gmsd = garmin_fs_alloc(-1);
-    waypt->fs.FsChainAdd(gmsd);
+      if (c.isNull()) {
+        categoryList << QString::asprintf("Category %d", i+1);
+      }
+//				*fout << QString::asprintf("%s", gps_categories[i]);
+      else {
+        categoryList << c;
+      }
+
+    }
+    categories = categories >> 1;
   }
-  garmin_fs_t::set_category(gmsd, cat);
-  return true;
+  return categoryList;
 }
diff --git a/garmin_fs.h b/garmin_fs.h
@@ -25,7 +25,9 @@
 #define GARMIN_FS_H
 
 #include <cstdint>     // for int32_t, int16_t, uint16_t
+#include <optional>    // for optional
 
+#include <QList>       // for QList
 #include <QString>     // for QString
 
 #include "defs.h"
@@ -41,9 +43,9 @@
 */
 
 struct garmin_ilink_t {
-  int ref_count;
-  double lat, lon, alt;
-  garmin_ilink_t* next;
+  double lat;
+  double lon;
+  double alt;
 };
 
 struct garmin_fs_flags_t {
@@ -95,6 +97,8 @@ struct garmin_fs_flags_t {
 
 class garmin_fs_t : public FormatSpecificData {
 public:
+  /* Data Members */
+
   garmin_fs_flags_t flags;
 
   int protocol{0};		/* ... used by device (-1 is MapSource) */
@@ -117,26 +121,30 @@ class garmin_fs_t : public FormatSpecificData {
   QString email;				/* email address */
   unsigned int duration; /* expected travel time to next route point, in seconds, only when auto-routed */
 
-  garmin_ilink_t* ilinks{nullptr};
+  QList<garmin_ilink_t> ilinks;
 #ifdef GMSD_EXPERIMENTAL
   char subclass[22]{};
 #endif
 
-public:
+  /* Special Member Functions */
+
   garmin_fs_t() : FormatSpecificData(kFsGmsd) {}
-private:
-  garmin_fs_t(const garmin_fs_t& other) = default;
-public:
-  garmin_fs_t& operator=(const garmin_fs_t& rhs) = delete; /* not implemented */
-  garmin_fs_t(garmin_fs_t&&) = delete; /* not implemented */
-  garmin_fs_t& operator=(garmin_fs_t&&) = delete; /* not implemented */
-  ~garmin_fs_t() override;
+  explicit garmin_fs_t(int p) : garmin_fs_t() {protocol = p;}
+
+  /* Member Functions */
+
+  garmin_fs_t* clone() const override
+  {
+    return new garmin_fs_t(*this);
+  }
 
-  garmin_fs_t* clone() const override;
   static garmin_fs_t* find(const Waypoint* wpt) {
     return reinterpret_cast<garmin_fs_t*>(wpt->fs.FsChainFind(kFsGmsd));
   }
 
+  static std::optional<uint16_t> convert_category(const QString& category_name);
+  static QStringList print_categories(uint16_t categories);
+
 #define GEN_GMSD_METHODS(field) \
   static bool has_##field(const garmin_fs_t* gmsd) \
   { \
@@ -212,18 +220,10 @@ class garmin_fs_t : public FormatSpecificData {
   GEN_GMSD_STR_METHODS(email)
 
 #undef GEN_GMSD_STR_METHODS
-};
-
-garmin_fs_t* garmin_fs_alloc(int protocol);
-void garmin_fs_destroy(void* fs);
-void garmin_fs_copy(void** dest, const void* src);
-
-/* ..convert_category: returns true=OK; false=Unable to convert category */
-bool garmin_fs_convert_category(const QString& category_name, uint16_t* category);
 
-/* ..merge_category: returns true=OK; false=Unable to convert category */
-bool garmin_fs_merge_category(const QString& category_name, Waypoint* waypt);
-
-#define GMSD_SECTION_CATEGORIES "Garmin Categories"
+private:
+  /* Constants */
 
+  static constexpr char kGmsdSectionCategories[] = "Garmin Categories";
+};
 #endif
diff --git a/garmin_gpi.cc b/garmin_gpi.cc
@@ -41,7 +41,7 @@
 
 #include "defs.h"
 #include "formspec.h"              // for FormatSpecificDataList
-#include "garmin_fs.h"             // for garmin_fs_t, garmin_fs_alloc
+#include "garmin_fs.h"             // for garmin_fs_t
 #include "gbfile.h"                // for gbfputint32, gbfgetint32, gbfgetint16, gbfputint16, gbfgetc, gbfputc, gbfread, gbftell, gbfwrite, gbfseek, gbfclose, gbfopen_le, gbfgetuint16, gbsize_t, gbfile
 #include "jeeps/gpsmath.h"         // for GPS_Math_Deg_To_Semi, GPS_Math_Semi_To_Deg
 
@@ -76,7 +76,7 @@ GarminGPIFormat::gpi_gmsd_init(Waypoint* wpt)
   }
   garmin_fs_t* gmsd = garmin_fs_t::find(wpt);
   if (gmsd == nullptr) {
-    gmsd = garmin_fs_alloc(-1);
+    gmsd = new garmin_fs_t(-1);
     wpt->fs.FsChainAdd(gmsd);
   }
   return gmsd;
@@ -700,7 +700,7 @@ GarminGPIFormat::wdata_compute_size(writer_data_t* data) const
   res = 23;  /* bounds, ... of tag 0x80008 */
 
   foreach (Waypoint* wpt, data->waypt_list) {
-    garmin_fs_t* gmsd;
+    const garmin_fs_t* gmsd;
 
     res += 12;    /* tag/sz/sub-sz */
     res += 19;    /* poi fixed size */