From 04593e8ec4bac2606dec54605c72a6a49cc83f9b Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sun, 24 Mar 2019 21:50:01 +0100 Subject: [PATCH] Cleanup: fix printGPSCoords signature and leaks The printGPSCoords() function returns a copied C-style string. Since the owndership is transferred to the caller, the correct return type is "char *" instead of "const char *". Thus a number of casts when calling free can be removed. Moreover a number of callers didn't free the string and thus were leaking memory. Fix them. Ultimately we might want two versions of the function: one for QString, one for C-style strings. Signed-off-by: Berthold Stoeger --- core/gpslocation.cpp | 4 +++- core/load-git.c | 4 ++-- core/parse-xml.c | 4 ++-- core/qthelper.cpp | 2 +- core/qthelper.h | 2 +- core/subsurface-qt/DiveObjectHelper.cpp | 7 ++++++- desktop-widgets/locationinformation.cpp | 8 ++++---- desktop-widgets/modeldelegates.cpp | 4 ++-- desktop-widgets/tab-widgets/maintab.cpp | 4 ++-- map-widget/qmlmapwidgethelper.cpp | 4 ++-- mobile-widgets/qmlmanager.cpp | 9 ++++++--- 11 files changed, 31 insertions(+), 21 deletions(-) diff --git a/core/gpslocation.cpp b/core/gpslocation.cpp index e0b176f58..d0bbcdc8f 100644 --- a/core/gpslocation.cpp +++ b/core/gpslocation.cpp @@ -137,7 +137,9 @@ QString GpsLocation::currentPosition() if (delta < 300) { // we can simply use the last position that we tracked gpsTracker gt = m_trackers.last(); - QString gpsString = printGPSCoords(>.location); + char *gps = printGPSCoords(>.location); + QString gpsString = gps; + free(gps); qDebug() << "returning last position" << gpsString; return gpsString; } else { diff --git a/core/load-git.c b/core/load-git.c index 08d9b7765..8417fc2e4 100644 --- a/core/load-git.c +++ b/core/load-git.c @@ -164,10 +164,10 @@ static void parse_dive_gps(char *line, struct membuffer *str, void *_dive) dive->dive_site = ds; } else { if (dive_site_has_gps_location(ds) && !same_location(&ds->location, &location)) { - const char *coords = printGPSCoords(&location); + char *coords = printGPSCoords(&location); // we have a dive site that already has GPS coordinates ds->notes = add_to_string(ds->notes, translate("gettextFromC", "multiple GPS locations for this dive site; also %s\n"), coords); - free((void *)coords); + free(coords); } ds->location = location; } diff --git a/core/parse-xml.c b/core/parse-xml.c index c2e2ef937..c118c89cc 100644 --- a/core/parse-xml.c +++ b/core/parse-xml.c @@ -1201,9 +1201,9 @@ static void gps_in_dive(char *buffer, struct dive *dive, struct parser_state *st fprintf(stderr, "dive site uuid in dive, but gps location (%10.6f/%10.6f) different from dive location (%10.6f/%10.6f)\n", ds->location.lat.udeg / 1000000.0, ds->location.lon.udeg / 1000000.0, location.lat.udeg / 1000000.0, location.lon.udeg / 1000000.0); - const char *coords = printGPSCoords(&location); + char *coords = printGPSCoords(&location); ds->notes = add_to_string(ds->notes, translate("gettextFromC", "multiple GPS locations for this dive site; also %s\n"), coords); - free((void *)coords); + free(coords); } else { ds->location = location; } diff --git a/core/qthelper.cpp b/core/qthelper.cpp index 89dd1befb..26a8a68f9 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -72,7 +72,7 @@ QString distance_string(int distanceInMeters) return str; } -extern "C" const char *printGPSCoords(const location_t *location) +extern "C" char *printGPSCoords(const location_t *location) { int lat = location->lat.udeg; int lon = location->lon.udeg; diff --git a/core/qthelper.h b/core/qthelper.h index 3e90a1979..ed09d70fb 100644 --- a/core/qthelper.h +++ b/core/qthelper.h @@ -123,7 +123,7 @@ void moveInVector(Vector &v, int rangeBegin, int rangeEnd, int destination) extern "C" { #endif -const char *printGPSCoords(const location_t *loc); +char *printGPSCoords(const location_t *loc); bool in_planner(); bool getProxyString(char **buffer); bool canReachCloudServer(); diff --git a/core/subsurface-qt/DiveObjectHelper.cpp b/core/subsurface-qt/DiveObjectHelper.cpp index 0742e382f..921d55bea 100644 --- a/core/subsurface-qt/DiveObjectHelper.cpp +++ b/core/subsurface-qt/DiveObjectHelper.cpp @@ -112,7 +112,12 @@ QString DiveObjectHelper::location() const QString DiveObjectHelper::gps() const { struct dive_site *ds = m_dive->dive_site; - return ds ? QString(printGPSCoords(&ds->location)) : QString(); + if (!ds) + return QString(); + char *gps = printGPSCoords(&ds->location); + QString res = gps; + free(gps); + return res; } QString DiveObjectHelper::gps_decimal() const diff --git a/desktop-widgets/locationinformation.cpp b/desktop-widgets/locationinformation.cpp index 7356f8370..f4da411af 100644 --- a/desktop-widgets/locationinformation.cpp +++ b/desktop-widgets/locationinformation.cpp @@ -114,9 +114,9 @@ void LocationInformationWidget::updateLabels() else ui.diveSiteNotes->clear(); if (has_location(&diveSite->location)) { - const char *coords = printGPSCoords(&diveSite->location); + char *coords = printGPSCoords(&diveSite->location); ui.diveSiteCoordinates->setText(coords); - free((void *)coords); + free(coords); } else { ui.diveSiteCoordinates->clear(); } @@ -138,10 +138,10 @@ void LocationInformationWidget::updateGpsCoordinates(const location_t &location) { QString oldText = ui.diveSiteCoordinates->text(); - const char *coords = printGPSCoords(&location); + char *coords = printGPSCoords(&location); ui.diveSiteCoordinates->setText(coords); enableLocationButtons(has_location(&location)); - free((void *)coords); + free(coords); if (oldText != ui.diveSiteCoordinates->text()) markChangedWidget(ui.diveSiteCoordinates); } diff --git a/desktop-widgets/modeldelegates.cpp b/desktop-widgets/modeldelegates.cpp index a237c1306..c6405a433 100644 --- a/desktop-widgets/modeldelegates.cpp +++ b/desktop-widgets/modeldelegates.cpp @@ -484,9 +484,9 @@ void LocationFilterDelegate::paint(QPainter *painter, const QStyleOptionViewItem } if (bottomText.isEmpty()) { - const char *gpsCoords = printGPSCoords(&ds->location); + char *gpsCoords = printGPSCoords(&ds->location); bottomText = QString(gpsCoords); - free( (void*) gpsCoords); + free(gpsCoords); } if (dive_site_has_gps_location(ds) && currentDiveSiteHasGPS) { diff --git a/desktop-widgets/tab-widgets/maintab.cpp b/desktop-widgets/tab-widgets/maintab.cpp index 11ad99d50..a2ff15368 100644 --- a/desktop-widgets/tab-widgets/maintab.cpp +++ b/desktop-widgets/tab-widgets/maintab.cpp @@ -411,9 +411,9 @@ void MainTab::updateDiveInfo(bool clear) ui.locationTags->setText(constructLocationTags(&ds->taxonomy, true)); if (ui.locationTags->text().isEmpty() && has_location(&ds->location)) { - const char *coords = printGPSCoords(&ds->location); + char *coords = printGPSCoords(&ds->location); ui.locationTags->setText(coords); - free((void *)coords); + free(coords); } } else { ui.location->clear(); diff --git a/map-widget/qmlmapwidgethelper.cpp b/map-widget/qmlmapwidgethelper.cpp index a7fb86d9b..61b8f0abe 100644 --- a/map-widget/qmlmapwidgethelper.cpp +++ b/map-widget/qmlmapwidgethelper.cpp @@ -246,10 +246,10 @@ void MapWidgetHelper::copyToClipboardCoordinates(QGeoCoordinate coord, bool form bool savep = prefs.coordinates_traditional; prefs.coordinates_traditional = formatTraditional; location_t location = mk_location(coord); - const char *coordinates = printGPSCoords(&location); + char *coordinates = printGPSCoords(&location); QApplication::clipboard()->setText(QString(coordinates), QClipboard::Clipboard); - free((void *)coordinates); + free(coordinates); prefs.coordinates_traditional = savep; } diff --git a/mobile-widgets/qmlmanager.cpp b/mobile-widgets/qmlmanager.cpp index 8973aef43..d9da5e562 100644 --- a/mobile-widgets/qmlmanager.cpp +++ b/mobile-widgets/qmlmanager.cpp @@ -1602,9 +1602,12 @@ QString QMLManager::getGpsFromSiteName(const QString& siteName) struct dive_site *ds; ds = get_dive_site_by_name(qPrintable(siteName)); - if (ds) - return QString(printGPSCoords(&ds->location)); - return QString(); + if (!ds) + return QString(); + char *gps = printGPSCoords(&ds->location); + QString res = gps; + free(gps); + return res; } void QMLManager::setNotificationText(QString text)