Merge pull request #63 from maxmind/dave/fix-valgrind-issue

Fix a valgrind memcheck issue and check gettimeofday return value
maxmind · Jul 27, 2015 · 733b36e · 733b36e
2 parents 34d142a + f63ff8d
commit 733b36e
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 15 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -6,6 +6,11 @@
   GitHub #56.)
 * Fixed segfault when doing a lookup on an empty database. (Fix by NesoK.
   GitHub #62.)
+* Fixed a memcheck error from valgrind in the `_check_mtime`
+  function. (Reported by yurivct. GitHub #60.)
+* Fixed `_check_mtime` to check the return value of `gettimeofday` rather than
+  just assuming it worked.
+
 
 1.6.5 2015-02-25
 

diff --git a/apps/geoiplookup.c b/apps/geoiplookup.c
@@ -290,7 +290,7 @@ void geoiplookup(GeoIP * gi, char *hostname, int i)
             }
         }else if (GEOIP_COUNTRY_EDITION == i) {
             country_id = GeoIP_id_by_ipnum(gi, ipnum);
-            if (country_id < 0 || country_id >= (int) GeoIP_num_countries()) {
+            if (country_id < 0 || country_id >= (int)GeoIP_num_countries()) {
                 printf("%s: Invalid database\n", GeoIPDBDescription[i]);
                 return;
             }

diff --git a/apps/geoiplookup6.c b/apps/geoiplookup6.c
@@ -177,7 +177,7 @@ geoiplookup(GeoIP * gi, char *hostname, int i)
             }
         }else if (GEOIP_COUNTRY_EDITION_V6 == i) {
             country_id = GeoIP_id_by_ipnum_v6(gi, ipnum);
-            if (country_id < 0 || country_id >= (int) GeoIP_num_countries()) {
+            if (country_id < 0 || country_id >= (int)GeoIP_num_countries()) {
                 printf("%s: Invalid database\n", GeoIPDBDescription[i]);
                 return;
             }

diff --git a/libGeoIP/GeoIP.c b/libGeoIP/GeoIP.c
@@ -1071,7 +1071,7 @@ static void _setup_segments(GeoIP * gi)
 }
 
 static
-int _check_mtime(GeoIP *gi)
+void _check_mtime(GeoIP *gi)
 {
     struct stat buf;
     ssize_t idx_size;
@@ -1088,9 +1088,15 @@ int _check_mtime(GeoIP *gi)
 #if !defined(_WIN32)
         /* stat only has second granularity, so don't
          * call it more than once a second */
-        gettimeofday(&t, NULL);
+        if (0 != gettimeofday(&t, NULL)) {
+            DEBUG_MSGF(gi->flags,
+                       "Error calling gettimeofday: %s\n",
+                       strerror(errno));
+            return;
+        }
+
         if (t.tv_sec == gi->last_mtime_check) {
-            return 0;
+            return;
         }
         gi->last_mtime_check = t.tv_sec;
 
@@ -1101,7 +1107,7 @@ int _check_mtime(GeoIP *gi)
         GetSystemTimeAsFileTime(&ft);
         t = FILETIME_TO_USEC(ft) / 1000 / 1000;
         if (t == gi->last_mtime_check) {
-            return 0;
+            return;
         }
         gi->last_mtime_check = t;
 
@@ -1131,7 +1137,7 @@ int _check_mtime(GeoIP *gi)
                             DEBUG_MSGF(gi->flags,
                                        "Out of memory when reloading %s\n",
                                        gi->file_path);
-                            return -1;
+                            return;
                         }
                     }
                 }
@@ -1142,7 +1148,7 @@ int _check_mtime(GeoIP *gi)
                     DEBUG_MSGF(gi->flags,
                                "Error Opening file %s when reloading\n",
                                gi->file_path);
-                    return -1;
+                    return;
                 }
                 gi->mtime = buf.st_mtime;
                 gi->size = buf.st_size;
@@ -1152,7 +1158,7 @@ int _check_mtime(GeoIP *gi)
                     DEBUG_MSGF(gi->flags,
                                "GEOIP_MMAP_CACHE is not supported on WIN32\n");
                     gi->cache = 0;
-                    return -1;
+                    return;
 #else
                     gi->cache =
                         mmap(NULL, buf.st_size, PROT_READ, MAP_PRIVATE, fileno(
@@ -1164,7 +1170,7 @@ int _check_mtime(GeoIP *gi)
                                    gi->file_path);
 
                         gi->cache = NULL;
-                        return -1;
+                        return;
                     }
 #endif
                 } else if (gi->flags & GEOIP_MEMORY_CACHE) {
@@ -1173,7 +1179,7 @@ int _check_mtime(GeoIP *gi)
                         DEBUG_MSGF(gi->flags,
                                    "Error reading file %s when reloading\n",
                                    gi->file_path);
-                        return -1;
+                        return;
                     }
                 }
 
@@ -1185,14 +1191,14 @@ int _check_mtime(GeoIP *gi)
                 if (gi->databaseSegments == NULL) {
                     DEBUG_MSGF(gi->flags, "Error reading file %s -- corrupt\n",
                                gi->file_path);
-                    return -1;
+                    return;
                 }
 
                 idx_size = get_index_size(gi, &buf);
                 if (idx_size < 0) {
                     DEBUG_MSGF(gi->flags, "Error file %s -- corrupt\n",
                                gi->file_path);
-                    return -1;
+                    return;
                 }
 
                 if (gi->flags & GEOIP_INDEX_CACHE) {
@@ -1205,14 +1211,14 @@ int _check_mtime(GeoIP *gi)
                                 gi->flags,
                                 "Error reading file %s where reloading\n",
                                 gi->file_path);
-                            return -1;
+                            return;
                         }
                     }
                 }
             }
         }
     }
-    return 0;
+    return;
 }
 
 #define ADDR_STR_LEN (8 * 4 + 7 + 1)
@@ -1586,6 +1592,9 @@ GeoIP * GeoIP_open(const char * filename, int flags)
     } else {
         gi->index_cache = NULL;
     }
+
+    gi->last_mtime_check = 0;
+
     return gi;
 }