[gexiv2] Fix GPS rational calculation

[Jens Georg <jensgeorg src gnome org>]

commit 058abb2a530ebc465ea994f144f0cf682c2ffc51
Author: Jens Georg <mail jensge org>
Date:   Fri Feb 24 23:03:20 2017 +0100

    Fix GPS rational calculation
    
    Apparently there are some images where num == den == 0 for the GPS fractions
    which caused the code to bail out too early and left a) wrong GPS values and
    b) caused get_gps_info to return 0 for all other values after the failing one.
    
    While we're at it, clean up the functions to be a bit more C++-y. Just because
    we export a C interface doesn't mean we can't use nice C++ internally.
    
    Signed-off-by: Jens Georg <mail jensge org>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=775249

 gexiv2/gexiv2-metadata-gps.cpp |  192 ++++++++++++++++++----------------------
 1 files changed, 86 insertions(+), 106 deletions(-)
---
diff --git a/gexiv2/gexiv2-metadata-gps.cpp b/gexiv2/gexiv2-metadata-gps.cpp
index 51f1495..43bb37e 100644
--- a/gexiv2/gexiv2-metadata-gps.cpp
+++ b/gexiv2/gexiv2-metadata-gps.cpp
@@ -16,20 +16,49 @@
 #include <glib-object.h>
 #include <exiv2/exif.hpp>
 
+#include <limits>
+
 G_BEGIN_DECLS
 
+static double convert_rational(const Exiv2::Rational& r) {
+    if (r.first == 0) {
+        return 0.0;
+    }
+
+    if (r.second == 0) {
+        throw std::overflow_error("Invalid fraction");
+    }
+
+    auto num = static_cast<double>(r.first);
+    auto den = static_cast<double>(r.second);
+
+    return num / den;
+}
+
+struct GPointer {
+    GPointer(gpointer p) : _p(p) {}
+    ~GPointer() { g_free(_p); }
+    gpointer reset(gpointer p) { gpointer old = p; std::swap(old, _p); return old; };
+
+    gpointer _p;
+private:
+    GPointer operator=(const GPointer &other);
+    GPointer(const GPointer &other);
+};
+
 gboolean gexiv2_metadata_get_gps_longitude (GExiv2Metadata *self, gdouble *longitude) {
     g_return_val_if_fail (GEXIV2_IS_METADATA (self), FALSE);
     g_return_val_if_fail (longitude != NULL, FALSE);
-    g_return_val_if_fail(self->priv->image.get() != NULL, FALSE);
+    g_return_val_if_fail (self->priv->image.get() != NULL, FALSE);
 
     try {
-        double num, den, min, sec;
+        double min, sec;
         *longitude = 0.0;
 
         gchar* longitude_ref = gexiv2_metadata_get_exif_tag_string (self, "Exif.GPSInfo.GPSLongitudeRef");
+        GPointer longitude_ref_guard(longitude_ref);
+
         if (longitude_ref == NULL || longitude_ref[0] == '\0') {
-            g_free(longitude_ref);
             return FALSE;
         }
 
@@ -37,44 +66,18 @@ gboolean gexiv2_metadata_get_gps_longitude (GExiv2Metadata *self, gdouble *longi
         Exiv2::ExifKey key ("Exif.GPSInfo.GPSLongitude");
         Exiv2::ExifData::iterator it = exif_data.findKey (key);
 
-        if (it != exif_data.end () && (*it).count() == 3) {
-            num = (double)((*it).toRational(0).first);
-            den = (double)((*it).toRational(0).second);
-
-            if (den == 0) {
-                g_free(longitude_ref);
-                return FALSE;
-            }
-
-            *longitude = num / den;
-
-            num = (double)((*it).toRational(1).first);
-            den = (double)((*it).toRational(1).second);
-
-            if (den == 0) {
-                g_free(longitude_ref);
-                return FALSE;
+        if (it != exif_data.end () && it->count() == 3) {
+            *longitude = convert_rational(it->toRational(0));
+            min = convert_rational(it->toRational(1));
+            if (min != -1.0) {
+                *longitude += min / 60.0;
             }
 
-            min = num/den;
-
-            if (min != -1.0)
-                *longitude = *longitude + min / 60.0;
-
-            num = (double)((*it).toRational(2).first);
-            den = (double)((*it).toRational(2).second);
-
-            if (den == 0) {
-                g_free(longitude_ref);
-                return FALSE;
+            sec = convert_rational(it->toRational(2));
+            if (sec != -1.0) {
+                *longitude += sec / 3600.0;
             }
-
-            sec = num/den;
-
-            if (sec != -1.0)
-                *longitude = *longitude + sec / 3600.0;
         } else {
-            g_free(longitude_ref);
             return FALSE;
         }
 
@@ -82,28 +85,29 @@ gboolean gexiv2_metadata_get_gps_longitude (GExiv2Metadata *self, gdouble *longi
         if (longitude_ref[0] == 'S' || longitude_ref[0] == 'W')
             *longitude *= -1.0;
 
-        g_free(longitude_ref);
-
         return TRUE;
     } catch (Exiv2::Error &e) {
         LOG_ERROR(e);
+    } catch (std::overflow_error &e) {
+        LOG_ERROR(e);
     }
 
     return FALSE;
 }
 
 gboolean gexiv2_metadata_get_gps_latitude (GExiv2Metadata *self, gdouble *latitude) {
-    g_return_val_if_fail(GEXIV2_IS_METADATA (self), FALSE);
-    g_return_val_if_fail(latitude != NULL, FALSE);
-    g_return_val_if_fail(self->priv->image.get() != NULL, FALSE);
+    g_return_val_if_fail (GEXIV2_IS_METADATA (self), FALSE);
+    g_return_val_if_fail (latitude != NULL, FALSE);
+    g_return_val_if_fail (self->priv->image.get() != NULL, FALSE);
 
     try {
-        double num, den, min, sec;
+        double min, sec;
         *latitude = 0.0;
 
         gchar* latitude_ref = gexiv2_metadata_get_exif_tag_string (self, "Exif.GPSInfo.GPSLatitudeRef");
+        GPointer latitude_ref_guard(latitude_ref);
+
         if (latitude_ref == NULL || latitude_ref[0] == '\0') {
-            g_free(latitude_ref);
             return FALSE;
         }
 
@@ -111,56 +115,30 @@ gboolean gexiv2_metadata_get_gps_latitude (GExiv2Metadata *self, gdouble *latitu
         Exiv2::ExifKey key ("Exif.GPSInfo.GPSLatitude");
         Exiv2::ExifData::iterator it = exif_data.findKey (key);
 
-        if (it != exif_data.end () && (*it).count() == 3) {
-            num = (double)((*it).toRational(0).first);
-            den = (double)((*it).toRational(0).second);
-
-            if (den == 0) {
-                g_free(latitude_ref);
-                return FALSE;
+        if (it != exif_data.end () && it->count() == 3) {
+            *latitude = convert_rational(it->toRational(0));
+            min = convert_rational(it->toRational(1));
+            if (min != -1.0) {
+                *latitude += min / 60.0;
             }
 
-            *latitude = num / den;
-
-            num = (double)((*it).toRational(1).first);
-            den = (double)((*it).toRational(1).second);
-
-            if (den == 0) {
-                g_free(latitude_ref);
-                return FALSE;
-            }
-
-            min = num/den;
-
-            if (min != -1.0)
-                *latitude = *latitude + min / 60.0;
-
-            num = (double)((*it).toRational(2).first);
-            den = (double)((*it).toRational(2).second);
-
-            if (den == 0) {
-                g_free(latitude_ref);
-                return FALSE;
+            sec = convert_rational(it->toRational(2));
+            if (sec != -1.0) {
+                *latitude += sec / 3600.0;
             }
-
-            sec = num/den;
-
-            if (sec != -1.0)
-                *latitude = *latitude + sec / 3600.0;
-        } else {
-            g_free(latitude_ref);
-            return FALSE;
-        }
+       } else {
+           return FALSE;
+       }
 
         // There's some weird stuff out there in the wild.
         if (latitude_ref[0] == 'S' || latitude_ref[0] == 'W')
             *latitude *= -1.0;
 
-        g_free(latitude_ref);
-
         return TRUE;
     } catch (Exiv2::Error &e) {
         LOG_ERROR(e);
+    } catch (std::overflow_error &e) {
+        LOG_ERROR(e);
     }
 
     return FALSE;
@@ -172,12 +150,12 @@ gboolean gexiv2_metadata_get_gps_altitude (GExiv2Metadata *self, gdouble *altitu
     g_return_val_if_fail(self->priv->image.get() != NULL, FALSE);
 
     try {
-        double num, den;
         *altitude = 0.0;
 
         gchar* altitude_ref = gexiv2_metadata_get_exif_tag_string (self, "Exif.GPSInfo.GPSAltitudeRef");
+        GPointer altitude_ref_guard(altitude_ref);
+
         if (altitude_ref == NULL || altitude_ref[0] == '\0') {
-            g_free(altitude_ref);
             return FALSE;
         }
 
@@ -185,29 +163,20 @@ gboolean gexiv2_metadata_get_gps_altitude (GExiv2Metadata *self, gdouble *altitu
         Exiv2::ExifKey key ("Exif.GPSInfo.GPSAltitude");
         Exiv2::ExifData::iterator it = exif_data.findKey (key);
 
-        if (it != exif_data.end () && (*it).count() == 1) {
-            num = (double)((*it).toRational(0).first);
-            den = (double)((*it).toRational(0).second);
-
-            if (den == 0) {
-                g_free(altitude_ref);
-                return FALSE;
-            }
-
-            *altitude = num/den;
+        if (it != exif_data.end () && it->count() == 1) {
+            *altitude = convert_rational(it->toRational(0));
         } else {
-            g_free(altitude_ref);
             return FALSE;
         }
 
         if (altitude_ref[0] == '1')
             *altitude *= -1.0;
 
-        g_free(altitude_ref);
-
         return TRUE;
     } catch (Exiv2::Error &e) {
         LOG_ERROR(e);
+    } catch (std::overflow_error &e) {
+        LOG_ERROR(e);
     }
 
     return FALSE;
@@ -215,16 +184,27 @@ gboolean gexiv2_metadata_get_gps_altitude (GExiv2Metadata *self, gdouble *altitu
 
 gboolean gexiv2_metadata_get_gps_info (GExiv2Metadata *self, gdouble *longitude, gdouble *latitude,
     gdouble *altitude) {
-    if ( ! gexiv2_metadata_get_gps_longitude (self, longitude))
-        return FALSE;
+    gboolean result = FALSE;
 
-    if ( ! gexiv2_metadata_get_gps_latitude (self, latitude))
-        return FALSE;
-        
-    if ( ! gexiv2_metadata_get_gps_altitude (self, altitude))
+    if (!gexiv2_metadata_get_gps_longitude (self, longitude)) {
+        *longitude = 0.0;
+    } else {
+        result = TRUE;
+    }
+
+    if (!gexiv2_metadata_get_gps_latitude (self, latitude)) {
+        *latitude = 0.0;
+    } else {
+        result = TRUE;
+    }
+
+    if (!gexiv2_metadata_get_gps_altitude (self, altitude)) {
         *altitude = 0.0;
-    
-    return TRUE;
+    } else {
+        result = TRUE;
+    }
+
+    return result;
 }