Merge lp:~epics-core/epics-base/epicsTime-status into lp:~epics-core/epics-base/3.16

[Andrew Johnson]

The return value epicsTimeERROR is useless when trying to track down a fault.
This branch replaces it with specific status values for the different kinds of errors.

[mdavidsaver (mdavidsaver)]

I like the idea of more specific errors, but I'd like to keep the definition of epicsTimeERROR to maintain source compatibility with external code (ie. the time provider in mrfioc2). Perhaps epicsTimeERROR can be an alias for an S_ error code (S_time_legacy?).

Otherwise no problem. I can make the change if Andrew approves a name.

review: Needs Fixing

[lp:~epics-core/epics-base/epicsTime-status]

12623. By Andrew Johnson on 2015-06-03

Show how to make time providers backwards-compatible

[Andrew Johnson (anj)]

The problem with keeping the epicsTimeERROR definition is that any code that equality compares it with a returned status value will stop working. Several such uses were fixed in this branch, for example this code would still compile with the requested change but needs to be modified to work properly:
  if (epicsTimeFromTimespec(&timeNow, &timespecNow) == epicsTimeERROR) { ... }
since epicsTimeFromTimespec() now returns S_time_conversion on error.

It could be rewritten as either
  if (epicsTimeFromTimespec(&timeNow, &timespecNow) != epicsTimeOK) { ... }
or
  if (epicsTimeFromTimespec(&timeNow, &timespecNow)) { ... }
both of which are backwards compatible.

Time providers that have to return a status and need to be backwards compatible could do this:

#include "epicsTime.h"
#ifndef M_time
/* S_time_... status values were not provided before Base 3.16 */
#define S_time_unsynchronized epicsTimeERROR
#define S_time_...whatever... epicsTimeERROR
#endif

I added the above suggestion for time provider code to the release notes.

[mdavidsaver (mdavidsaver)]

> The problem with keeping the epicsTimeERROR definition is that any code that equality compares it with a returned status value will stop working.

Good point.

FYI the case I have is:

https://github.com/epics-modules/mrfioc2/blob/master/evrApp/src/evrGTIF.cpp

The important thing is to have code which works for both <=3.15 and >=3.16, which could be done the in way you mention.

review: Approve

=== modified file 'documentation/RELEASE_NOTES.html'
--- documentation/RELEASE_NOTES.html	2015-04-16 23:12:59 +0000
+++ documentation/RELEASE_NOTES.html	2015-06-03 05:15:51 +0000
@@ -20,6 +20,28 @@
 
 -->
 
+<h3>epicsTime API return status</h3>
+
+<p>The epicsTime routines that used to return epicsTimeERROR now return a specific
+S_time_ status value, allowing the caller to discover the reason for any failure.
+The identifier <tt>epicsTimeERROR</tt> is no longer defined, so any references to
+it in source code will no longer compile. The identifier epicsTimeOK still exists
+and has the value 0 as before, so most code that uses these APIs can be changed in
+a way that is backwards-compatible with the previous return status.</p>
+
+<p>Time providers that have to return a status value and still need to be built
+with earlier versions of Base can define the necessary status symbols like this:</p>
+
+<blockquote><pre>
+#include "epicsTime.h"
+
+#ifndef M_time
+/* S_time_... status values were not provided before Base 3.16 */
+#define S_time_unsynchronized epicsTimeERROR
+#define S_time_...whatever... epicsTimeERROR
+#endif
+</pre></blockquote>
+
 <h3>Callback subsystem API</h3>
 
 <p>Added a new macro <tt>callbackGetPriority(prio, callback)</tt> to the
 
=== modified file 'src/libCom/error/Makefile'
--- src/libCom/error/Makefile	2014-08-01 22:55:53 +0000
+++ src/libCom/error/Makefile	2015-06-03 05:15:51 +0000
@@ -22,6 +22,7 @@
 # For bldErrSymTbl
 #
 ERR_S_FILES += $(LIBCOM)/osi/devLib.h
+ERR_S_FILES += $(LIBCOM)/osi/epicsTime.h
 ERR_S_FILES += $(LIBCOM)/as/asLib.h
 ERR_S_FILES += $(LIBCOM)/misc/epicsStdlib.h
 ERR_S_FILES += $(LIBCOM)/pool/epicsThreadPool.h
 
=== modified file 'src/libCom/error/errMdef.h'
--- src/libCom/error/errMdef.h	2014-08-01 22:55:53 +0000
+++ src/libCom/error/errMdef.h	2015-06-03 05:15:51 +0000
@@ -50,6 +50,7 @@
 #define M_gddFuncTbl	(526 <<16) /*gdd jump table*/
 #define M_stdlib        (527 <<16) /*EPICS Standard library*/
 #define M_pool          (528 <<16) /*Thread pool*/
+#define M_time          (529 <<16) /*epicsTime*/
 
 epicsShareFunc void epicsShareAPI errSymLookup(long status, char *pBuf, unsigned bufLength);
 epicsShareFunc void epicsShareAPI errSymTest(unsigned short modnum, unsigned short begErrNum, unsigned short endErrNum);
 
=== modified file 'src/libCom/osi/epicsGeneralTime.c'
--- src/libCom/osi/epicsGeneralTime.c	2013-12-11 23:50:29 +0000
+++ src/libCom/osi/epicsGeneralTime.c	2015-06-03 05:15:51 +0000
@@ -86,7 +86,7 @@
 int generalTimeGetExceptPriority(epicsTimeStamp *pDest, int *pPrio, int ignore)
 {
     gtProvider *ptp;
-    int status = epicsTimeERROR;
+    int status = S_time_noProvider;
 
     generalTime_Init();
 
@@ -107,6 +107,7 @@
                     *pPrio = ptp->priority;
             } else {
                 int key;
+
                 *pDest = gtPvt.lastProvidedTime;
                 if (pPrio)
                     *pPrio = gtPvt.lastTimeProvider->priority;
@@ -117,8 +118,7 @@
             break;
         }
     }
-    if (status == epicsTimeERROR &&
-        ignore == 0)
+    if (status && ignore == 0)
         gtPvt.lastTimeProvider = NULL;
     epicsMutexUnlock(gtPvt.timeListLock);
 
@@ -135,7 +135,8 @@
     gtProvider *ptp = gtPvt.lastTimeProvider;
 
     if (ptp == NULL ||
-        ptp->getInt.Time == NULL) return epicsTimeERROR;
+        ptp->getInt.Time == NULL)
+	return S_time_noProvider;
 
     return ptp->getInt.Time(pDest);
 }
@@ -145,20 +146,20 @@
     int *pPrio)
 {
     gtProvider *ptp;
-    int status = epicsTimeERROR;
+    int status = S_time_noProvider;
 
     generalTime_Init();
 
     if ((eventNumber < 0 || eventNumber >= NUM_TIME_EVENTS) &&
         (eventNumber != epicsTimeEventBestTime))
-        return status;
+        return S_time_badEvent;
 
     epicsMutexMustLock(gtPvt.eventListLock);
     for (ptp = (gtProvider *)ellFirst(&gtPvt.eventProviders);
          ptp; ptp = (gtProvider *)ellNext(&ptp->node)) {
 
         status = ptp->get.Event(pDest, eventNumber);
-        if (status != epicsTimeERROR) {
+        if (status == epicsTimeOK) {
             gtPvt.lastEventProvider = ptp;
             if (pPrio)
                 *pPrio = ptp->priority;
@@ -169,6 +170,7 @@
                     gtPvt.lastProvidedBestTime = *pDest;
                 } else {
                     int key;
+
                     *pDest = gtPvt.lastProvidedBestTime;
                     key = epicsInterruptLock();
                     gtPvt.ErrorCounts++;
@@ -180,6 +182,7 @@
                     gtPvt.eventTime[eventNumber] = *pDest;
                 } else {
                     int key;
+
                     *pDest = gtPvt.eventTime[eventNumber];
                     key = epicsInterruptLock();
                     gtPvt.ErrorCounts++;
@@ -189,7 +192,7 @@
             break;
         }
     }
-    if (status == epicsTimeERROR)
+    if (status)
         gtPvt.lastEventProvider = NULL;
     epicsMutexUnlock(gtPvt.eventListLock);
 
@@ -213,7 +216,8 @@
         gtProvider *ptp = gtPvt.lastEventProvider;
 
         if (ptp == NULL ||
-            ptp->getInt.Event == NULL) return epicsTimeERROR;
+            ptp->getInt.Event == NULL)
+	    return S_time_noProvider;
 
         return ptp->getInt.Event(pDest, eventNumber);
     }
@@ -271,11 +275,11 @@
     generalTime_Init();
 
     if (name == NULL || getEvent == NULL)
-        return epicsTimeERROR;
+        return S_time_badArgs;
 
     ptp = (gtProvider *)malloc(sizeof(gtProvider));
     if (ptp == NULL)
-        return epicsTimeERROR;
+        return S_time_noMemory;
 
     ptp->name         = epicsStrDup(name);
     ptp->priority     = priority;
@@ -293,7 +297,7 @@
     gtProvider *ptp = findProvider(&gtPvt.eventProviders, gtPvt.eventListLock,
         name, priority);
     if (ptp == NULL)
-        return epicsTimeERROR;
+        return S_time_noProvider;
 
     ptp->getInt.Event = getEvent;
 
@@ -308,11 +312,11 @@
     generalTime_Init();
 
     if (name == NULL || getTime == NULL)
-        return epicsTimeERROR;
+        return S_time_badArgs;
 
     ptp = (gtProvider *)malloc(sizeof(gtProvider));
     if (ptp == NULL)
-        return epicsTimeERROR;
+        return S_time_noMemory;
 
     ptp->name        = epicsStrDup(name);
     ptp->priority    = priority;
@@ -330,7 +334,7 @@
     gtProvider *ptp = findProvider(&gtPvt.timeProviders, gtPvt.timeListLock,
         name, priority);
     if (ptp == NULL)
-        return epicsTimeERROR;
+        return S_time_noProvider;
 
     ptp->getInt.Time = getTime;
 
@@ -388,7 +392,7 @@
         if (!message) {
             epicsMutexUnlock(gtPvt.timeListLock);
             printf("Out of memory\n");
-            return epicsTimeERROR;
+            return S_time_noMemory;
         }
 
         pout = message;
@@ -399,7 +403,7 @@
                 ptp->name, ptp->priority);
             if (level) {
                 epicsTimeStamp tempTS;
-                if (ptp->get.Time(&tempTS) != epicsTimeERROR) {
+                if (ptp->get.Time(&tempTS) == epicsTimeOK) {
                     char tempTSText[40];
                     epicsTimeToStrftime(tempTSText, sizeof(tempTSText),
                         "%Y-%m-%d %H:%M:%S.%06f", &tempTS);
@@ -430,7 +434,7 @@
         if (!message) {
             epicsMutexUnlock(gtPvt.eventListLock);
             printf("Out of memory\n");
-            return epicsTimeERROR;
+            return S_time_noMemory;
         }
 
         pout = message;
 
=== modified file 'src/libCom/osi/epicsTime.cpp'
--- src/libCom/osi/epicsTime.cpp	2014-08-25 21:27:18 +0000
+++ src/libCom/osi/epicsTime.cpp	2015-06-03 05:15:51 +0000
@@ -284,7 +284,7 @@
     local_tm_nano_sec tm;
 
     int status = epicsTime_localtime ( &ansiTimeTicks.ts, &tm.ansi_tm );
-    if ( status != epicsTimeOK ) {
+    if ( status ) {
         throw std::logic_error ( "epicsTime_localtime failed" );
     }
 
@@ -303,7 +303,7 @@
     gm_tm_nano_sec tm;
 
     int status = epicsTime_gmtime ( &ansiTimeTicks.ts, &tm.ansi_tm );
-    if ( status != epicsTimeOK ) {
+    if ( status ) {
         throw std::logic_error ( "epicsTime_gmtime failed" );
     }
 
@@ -891,7 +891,7 @@
             *pDest = dst.ts;
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
@@ -903,7 +903,7 @@
             *pDest = epicsTime ( dst );
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
@@ -915,7 +915,7 @@
             *pNSecDest = tmns.nSec;
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
@@ -927,7 +927,7 @@
             *pNSecDest = gmtmns.nSec;
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
@@ -940,7 +940,7 @@
             *pDest = epicsTime (tmns);
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
@@ -950,7 +950,7 @@
             *pDest = epicsTime (*pSrc);
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
@@ -960,7 +960,7 @@
             *pDest = epicsTime (*pSrc);
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
@@ -970,7 +970,7 @@
             *pDest = epicsTime (*pSrc);
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
@@ -980,7 +980,7 @@
             *pDest = epicsTime (*pSrc);
         }
         catch (...) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         return epicsTimeOK;
     }
 
=== modified file 'src/libCom/osi/epicsTime.h'
--- src/libCom/osi/epicsTime.h	2014-08-19 03:55:07 +0000
+++ src/libCom/osi/epicsTime.h	2015-06-03 05:15:51 +0000
@@ -17,6 +17,7 @@
 #include "shareLib.h"
 #include "epicsTypes.h"
 #include "osdTime.h"
+#include "errMdef.h"
 
 /* The EPICS Epoch is 00:00:00 Jan 1, 1990 UTC */
 #define POSIX_TIME_AT_EPICS_EPOCH 631152000u
@@ -170,9 +171,15 @@
 extern "C" {
 #endif /* __cplusplus */
 
-/* All epicsTime routines return (-1,0) for (failure,success) */
+/* epicsTime routines return S_time_ error status values */
 #define epicsTimeOK 0
-#define epicsTimeERROR (-1)
+#define S_time_noProvider       (M_time| 1) /*No time provider*/
+#define S_time_badEvent         (M_time| 2) /*Bad event number*/
+#define S_time_badArgs          (M_time| 3) /*Invalid arguments*/
+#define S_time_noMemory         (M_time| 4) /*Out of memory*/
+#define S_time_unsynchronized   (M_time| 5) /*Provider not synchronized*/
+#define S_time_timezone         (M_time| 6) /*Invalid timeone*/
+#define S_time_conversion       (M_time| 7) /*Time conversion error*/
 
 /*Some special values for eventNumber*/
 #define epicsTimeEventCurrentTime 0
 
=== modified file 'src/libCom/osi/os/Darwin/osdTime.cpp'
--- src/libCom/osi/os/Darwin/osdTime.cpp	2013-03-13 19:48:34 +0000
+++ src/libCom/osi/os/Darwin/osdTime.cpp	2015-06-03 05:15:51 +0000
@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <errno.h>
 #include <mach/mach.h>
 #include <mach/clock.h>
 
@@ -52,13 +53,13 @@
 int epicsTime_gmtime(const time_t *pAnsiTime, struct tm *pTM)
 {
     return gmtime_r(pAnsiTime, pTM) ?
-        epicsTimeOK : epicsTimeERROR;
+        epicsTimeOK : errno;
 }
 
 int epicsTime_localtime(const time_t *clock, struct tm *result)
 {
     return localtime_r(clock, result) ?
-        epicsTimeOK : epicsTimeERROR;
+        epicsTimeOK : errno;
 }
 
 extern "C" epicsShareFunc void
 
=== modified file 'src/libCom/osi/os/RTEMS/osdTime.cpp'
--- src/libCom/osi/os/RTEMS/osdTime.cpp	2011-08-23 18:20:00 +0000
+++ src/libCom/osi/os/RTEMS/osdTime.cpp	2015-06-03 05:15:51 +0000
@@ -91,7 +91,7 @@
         return epicsTimeOK;
     }
     else {
-        return epicsTimeERROR;
+        return errno;
     }
 }
 
@@ -102,7 +102,7 @@
         return epicsTimeOK;
     }
     else {
-        return epicsTimeERROR;
+        return errno;
     }
 }
 
 
=== modified file 'src/libCom/osi/os/WIN32/osdTime.cpp'
--- src/libCom/osi/os/WIN32/osdTime.cpp	2015-03-13 16:50:26 +0000
+++ src/libCom/osi/os/WIN32/osdTime.cpp	2015-06-03 05:15:51 +0000
@@ -164,7 +164,7 @@
     SYSTEMTIME st;
     BOOL status = FileTimeToSystemTime ( &ft, &st );
     if ( ! status ) {
-        return epicsTimeERROR;
+        return S_time_conversion;
     }
 
     pTM->tm_sec = st.wSecond; // seconds after the minute - [0,59]
@@ -193,7 +193,7 @@
     TIME_ZONE_INFORMATION tzInfo;
     DWORD tzStatus = GetTimeZoneInformation ( & tzInfo );
     if ( tzStatus == TIME_ZONE_ID_INVALID ) {
-        return epicsTimeERROR;
+        return S_time_timezone;
     }
 
     //
@@ -204,13 +204,13 @@
     SYSTEMTIME st;
     BOOL success = FileTimeToSystemTime ( & ft, & st );
     if ( ! success ) {
-        return epicsTimeERROR;
+        return S_time_conversion;
     }
     SYSTEMTIME lst;
     success = SystemTimeToTzSpecificLocalTime (
         & tzInfo, & st, & lst );
     if ( ! success ) {
-        return epicsTimeERROR;
+        return S_time_conversion;
     }
 
     //
@@ -220,7 +220,7 @@
     FILETIME lft;
     success = SystemTimeToFileTime ( & lst, & lft );
     if ( ! success ) {
-        return epicsTimeERROR;
+        return S_time_conversion;
     }
 
     int is_dst = -1; // unknown state of dst
@@ -234,14 +234,14 @@
         success = SystemTimeToFileTime (
             & tzInfo.StandardDate, & StandardDateFT );
         if ( ! success ) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         tzInfo.DaylightDate.wYear = st.wYear;
         FILETIME DaylightDateFT;
         success = SystemTimeToFileTime (
             & tzInfo.DaylightDate, & DaylightDateFT );
         if ( ! success ) {
-            return epicsTimeERROR;
+            return S_time_conversion;
         }
         if ( CompareFileTime ( & lft, & DaylightDateFT ) >= 0
                 && CompareFileTime ( & lft, & StandardDateFT ) < 0 ) {
 
=== modified file 'src/libCom/osi/os/posix/osdTime.cpp'
--- src/libCom/osi/os/posix/osdTime.cpp	2013-03-15 20:23:55 +0000
+++ src/libCom/osi/os/posix/osdTime.cpp	2015-06-03 05:15:51 +0000
@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <errno.h>
 
 #include "osiSock.h"
 
@@ -36,7 +37,7 @@
         struct timezone tz;
 
         if (gettimeofday (&tv, &tz))
-            return epicsTimeERROR;
+            return errno;
 
         *pDest = epicsTime(tv);
         return epicsTimeOK;
@@ -68,7 +69,7 @@
         return epicsTimeOK;
     }
     else {
-        return epicsTimeERROR;
+        return errno;
     }
 }
 
@@ -80,7 +81,7 @@
         return epicsTimeOK;
     }
     else {
-        return epicsTimeERROR;
+        return errno;
     }
 }
 
 
=== modified file 'src/libCom/osi/osiNTPTime.c'
--- src/libCom/osi/osiNTPTime.c	2014-08-25 21:27:18 +0000
+++ src/libCom/osi/osiNTPTime.c	2015-06-03 05:15:51 +0000
@@ -164,7 +164,7 @@
         }
 
         if (timespecNow.tv_sec <= POSIX_TIME_AT_EPICS_EPOCH ||
-            epicsTimeFromTimespec(&timeNow, &timespecNow) == epicsTimeERROR) {
+            epicsTimeFromTimespec(&timeNow, &timespecNow) != epicsTimeOK) {
             errlogPrintf("NTPTimeSync: Bad time received from NTP server\n");
             NTPTimePvt.synchronized = 0;
             continue;
@@ -213,7 +213,7 @@
     epicsUInt32 ticksSince;
 
     if (!NTPTimePvt.synchronized)
-        return epicsTimeERROR;
+        return S_time_unsynchronized;
 
     epicsMutexMustLock(NTPTimePvt.lock);
 
 
=== modified file 'src/std/dev/devGeneralTime.c'
--- src/std/dev/devGeneralTime.c	2013-12-17 18:54:04 +0000
+++ src/std/dev/devGeneralTime.c	2015-06-03 05:15:51 +0000
@@ -37,7 +37,7 @@
 {
     epicsTimeStamp ts;
 
-    if (epicsTimeERROR != epicsTimeGetCurrent(&ts)) {
+    if (epicsTimeOK == epicsTimeGetCurrent(&ts)) {
         *pseconds = ts.secPastEpoch + ((double)(ts.nsec)) * 1e-9;
         return 0;
     }