Jvinniec / CppEphem

CppEphem is a self-contained ephemeris package written in C++. It allows computation of celestial coordinate and date conversions and planet ephemerides by leveraging the Standards of Fundamental Astronomy (SOFA) software (included in the repo). It is C++11 compatible.
2 stars 2 forks source link

cirs2obs seg fault #6

Closed A-j-K closed 5 years ago

A-j-K commented 5 years ago

When trying out cirs2obs for the first time it seg faulted.

Fixed with:-

--- cirs2obs.cpp.orin   2019-01-30 10:28:26.240923590 +0000
+++ cirs2obs.cpp    2019-01-30 10:27:29.544922676 +0000
@@ -72,8 +72,9 @@

     // Set the current time as the default date for query
     time_t current_time = time(NULL) ;
-    struct tm * timeinfo;
-    gmtime_r(&current_time, timeinfo) ;
+    struct tm * timeinfo, tm_tmp;
+    gmtime_r(&current_time, &tm_tmp) ;
+    timeinfo = &tm_tmp;
     double dayfrac = (timeinfo->tm_hour + (timeinfo->tm_min/60.0) + ((timeinfo->tm_sec)/3600.0))/24.0 ;
     CEDate date({timeinfo->tm_year+1900.0, timeinfo->tm_mon+1.0, 1.0*timeinfo->tm_mday, dayfrac}) ;
Jvinniec commented 5 years ago

Nice catch! There's a somewhat easier fix, that avoids the temporary variable altogether.

--- a/src/cirs2obs.cpp
+++ b/src/cirs2obs.cpp
@@ -72,10 +72,10 @@ std::map<std::string, double> defaultoptions()

     // Set the current time as the default date for query
     time_t current_time = time(NULL) ;
-    struct tm * timeinfo;
-    gmtime_r(&current_time, timeinfo) ;
-    double dayfrac = (timeinfo->tm_hour + (timeinfo->tm_min/60.0) + ((timeinfo->tm_sec)/3600.0))/24.0 ;
-    CEDate date({timeinfo->tm_year+1900.0, timeinfo->tm_mon+1.0, 1.0*timeinfo->tm_mday, dayfrac}) ;
+    struct tm timeinfo;
+    gmtime_r(&current_time, &timeinfo) ;
+    double dayfrac = (timeinfo.tm_hour + (timeinfo.tm_min/60.0) + ((timeinfo.tm_sec)/3600.0))/24.0 ;
+    CEDate date({timeinfo.tm_year+1900.0, timeinfo.tm_mon+1.0, 1.0*timeinfo.tm_mday, dayfrac}) ;

Some quick checking also reveals that this is an issue in gal2obs as well, so I've made the same change there as well.

If you want to checkout the patch branch it's 6_cirs2obs_segfault_fix. Otherwise I'll merge this into the master once I verify the results and put out a patch release.

A-j-K commented 5 years ago

I saw the "correct, easier" fix but I just did the minimum to test it was that. Your's is the correct fix :) branch 6_cirs2obs_segfault_fix confirmed.