Merge lp:~khkim/epics-base/compression-3.15 into lp:~epics-core/epics-base/3.15

[Kim, Kukhee]

- Implement LIFO behavior in compress record
- Prepare new field BALG (Buffering Algorithm) to allow to choose FIFO and LIFO

[lp:~khkim/epics-base/compression-3.15]

12520. By Kim, Kukhee on 2014-08-29

add epics thread id validate routine and check the thread id to avoid iocshell crash for epicsThreadResume() command

12521. By Kim, Kukhee on 2014-08-29

add epics thread id validate routine

12522. By Kim, Kukhee on 2014-08-29

add epics thread id validate routine

12523. By Kim, Kukhee on 2014-08-29

add epics thread id validate routine - dummy, the validate function need to be complete

[Kim, Kukhee (khkim)]

Revision 12520 - 12523

Add epics thread id validation routine
Modify epicsThreadGetName to check up if the thread id is valid to avoid crash
Modify epicsThreadResume() command to avoid crash when user inputs an invalid thread id

[mdavidsaver (mdavidsaver)]

What should be done about switching between FIFO and LIFO at runtime? At present LIFO -> FIFO clears the buffer. FIFO -> LIFO can't clear the buffer, but could zero it. Alternately, make the BALG field read only somehow.

Otherwise this works as I expect.

To save Andrew the trouble, please add to RELEASE_NOTES.html

review: Needs Fixing

[lp:~khkim/epics-base/compression-3.15]

12524. By Kim, Kukhee on 2015-07-09

fill zeros into buffer when BALG is changed to LIFO from FIFO.

[mdavidsaver (mdavidsaver)]

Zeroing the buffer on reset seems sufficient.

As for epicsThreadValidateId(), it seems reasonable for the current implementation of epicsThreadResume(). IMO epicsThreadResume() is broken by design and should be removed, or epicsThread needs to be reimplemented around it. But this is the subject of another development.

Also, I'm commenting now because I've just noticed that there was activity on this branch. Since launchpad is sparing with email notifications it's a good idea to send a note to core-talk when pushing changes to a branch after a long period of inactivity.

review: Approve

[Ralph Lange (ralph-lange)]

Should have some tests added, shouldn't it?

review: Needs Fixing

[Andrew Johnson (anj)]

Results of AJ+MD+RL discussion about this today:
* This should be a 3.16 change (3.16.0.2 maybe), not 3.15.
* Needs Release Notes entry and POD documentation for the new field (AJ?).
* Remove the epicsThreadValidateId() changes, not ready.
* Automated tests would be nice given the complexity of the record, but we will accept the change without them.

Unmerged revisions

12524. By Kim, Kukhee on 2015-07-09

fill zeros into buffer when BALG is changed to LIFO from FIFO.

12523. By Kim, Kukhee on 2014-08-29

add epics thread id validate routine - dummy, the validate function need to be complete

12522. By Kim, Kukhee on 2014-08-29

add epics thread id validate routine

12521. By Kim, Kukhee on 2014-08-29

add epics thread id validate routine

12520. By Kim, Kukhee on 2014-08-29

add epics thread id validate routine and check the thread id to avoid iocshell crash for epicsThreadResume() command

12519. By Kim, Kukhee on 2014-08-28

Implement LIFO behavior in compress record

=== modified file 'src/libCom/iocsh/libComRegister.c'
--- src/libCom/iocsh/libComRegister.c	2014-10-31 17:19:09 +0000
+++ src/libCom/iocsh/libComRegister.c	2015-07-09 21:24:24 +0000
@@ -328,9 +328,8 @@
         }
         else {
             tid =(epicsThreadId)ltmp;
-            epicsThreadGetName(tid, nameBuf, sizeof nameBuf);
-            if (nameBuf[0] == '\0') {
-                fprintf(stderr, "'%s' is not a valid thread id\n", cp);
+            if(!epicsThreadValidateId(tid)) {
+                fprintf(stderr, "%s' is not a valid thread id\n", cp);
                 continue;
             }
 

=== modified file 'src/libCom/osi/epicsThread.h'
--- src/libCom/osi/epicsThread.h	2014-08-25 21:27:18 +0000
+++ src/libCom/osi/epicsThread.h	2015-07-09 21:24:24 +0000
@@ -94,6 +94,7 @@
 /* Failure results in an empty string stored in name */
 epicsShareFunc void epicsShareAPI epicsThreadGetName(
     epicsThreadId id, char *name, size_t size);
+epicsShareFunc epicsThreadId epicsShareAPI epicsThreadValidateId(const epicsThreadId id);
 
 epicsShareFunc int epicsShareAPI epicsThreadIsOkToBlock(void);
 epicsShareFunc void epicsShareAPI epicsThreadSetOkToBlock(int isOkToBlock);

=== modified file 'src/libCom/osi/os/RTEMS/osdThread.c'
--- src/libCom/osi/os/RTEMS/osdThread.c	2013-03-28 14:03:29 +0000
+++ src/libCom/osi/os/RTEMS/osdThread.c	2015-07-09 21:24:24 +0000
@@ -455,6 +455,13 @@
     name[size-1] = '\0';
 }
 
+
+/* the thread id validation routine needs to be completed */
+epicsThreadId  epicsThreadValidateId(epicsThreadId id)
+{
+    return id;
+}
+
 epicsThreadId epicsThreadGetId (const char *name)
 {
     struct taskVar *v;

=== modified file 'src/libCom/osi/os/WIN32/osdThread.c'
--- src/libCom/osi/os/WIN32/osdThread.c	2014-08-27 00:14:35 +0000
+++ src/libCom/osi/os/WIN32/osdThread.c	2015-07-09 21:24:24 +0000
@@ -894,6 +894,12 @@
     return "anonymous";
 }
 
+/* the thread id validation routine need to be completed */
+epicsShareFunc epicsThreadId epicsShareAPI epicsThreadValidateId(epicsThreadId id)
+{
+    return id;
+}
+
 /*
  * epicsThreadGetName ()
  */
@@ -902,6 +908,11 @@
 {
     win32ThreadParam * pParm = ( win32ThreadParam * ) id;
 
+    if(!epicsThreadValidateId(id)) {
+        pName[0] = '\0';
+        return;
+    }
+
     if ( size ) {
         size_t sizeMinusOne = size-1;
         strncpy ( pName, pParm->pName, sizeMinusOne );

=== modified file 'src/libCom/osi/os/posix/osdThread.c'
--- src/libCom/osi/os/posix/osdThread.c	2015-03-19 15:22:15 +0000
+++ src/libCom/osi/os/posix/osdThread.c	2015-07-09 21:24:24 +0000
@@ -763,9 +763,27 @@
 epicsShareFunc void epicsShareAPI epicsThreadGetName(epicsThreadId pthreadInfo, char *name, size_t size)
 {
     assert(epicsThreadOnceCalled);
+
+    if(!epicsThreadValidateId(pthreadInfo)) {
+        name[0] = '\0';
+        return;
+    }
+
     strncpy(name, pthreadInfo->name, size-1);
     name[size-1] = '\0';
 }
+
+epicsShareFunc epicsThreadId epicsShareAPI epicsThreadValidateId(epicsThreadId id)
+{
+    epicsThreadOSD *pthreadInfo;
+
+    pthreadInfo = (epicsThreadOSD *)ellFirst(&pthreadList);
+    while(pthreadInfo) {
+        if(pthreadInfo == (epicsThreadOSD *)id) break;
+        pthreadInfo = (epicsThreadOSD *) ellNext(&pthreadInfo->node);
+    }
+    return(pthreadInfo);
+}
 

 epicsShareFunc void epicsThreadMap(EPICS_THREAD_HOOK_ROUTINE func)
 {

=== modified file 'src/libCom/osi/os/vxWorks/osdThread.c'
--- src/libCom/osi/os/vxWorks/osdThread.c	2014-05-19 14:44:05 +0000
+++ src/libCom/osi/os/vxWorks/osdThread.c	2015-07-09 21:24:24 +0000
@@ -327,9 +327,21 @@
     return taskName(taskIdSelf());
 }
 
+/* the thread id validation routine needs to be completed */
+epicsThreadId epicsThreadValidateId(epicsThreadId id)
+{
+    return id;
+}
+
 void epicsThreadGetName (epicsThreadId id, char *name, size_t size)
 {
     int tid = (int)id;
+
+    if(!epicsThreadValidateId(id)) {
+        name[0] = '\0';
+        return;
+    }
+
     strncpy(name,taskName(tid),size-1);
     name[size-1] = '\0';
 }

=== modified file 'src/std/rec/compressRecord.c'
--- src/std/rec/compressRecord.c	2013-12-17 18:54:04 +0000
+++ src/std/rec/compressRecord.c	2015-07-09 21:24:24 +0000
@@ -92,6 +92,8 @@
     if (prec->alg == compressALG_Average && prec->sptr == 0){
         prec->sptr = calloc(prec->nsam, sizeof(double));
     }
+
+   if(prec->bptr && prec->nsam) memset(prec->bptr, 0, prec->nsam * sizeof(double));
 }
 
 static void monitor(compressRecord *prec)
@@ -106,7 +108,7 @@
     db_post_events(prec, prec->bptr, monitor_mask);
 }
 
-static void put_value(compressRecord *prec, double *psource, int n)
+static void put_value_FIFO(compressRecord *prec, double *psource, int n)
 {
     epicsInt32 offset = prec->off;
     epicsInt32 nuse = prec->nuse;
@@ -130,6 +132,42 @@
     prec->nuse = nuse;
     return;
 }
+
+static void put_value_LIFO(compressRecord *prec, double *psource, int n)
+{
+    epicsInt32 offset = prec->off;
+    epicsInt32 nuse = prec->nuse;
+    epicsInt32 nsam = prec->nsam; 
+    epicsInt32 start = offset - nuse;
+    double *pdest;
+    int i;
+    if(start<0) start += nsam;
+
+    for(i = 0; i < n; i++) {
+        if(--start < 0) start += nsam;
+        pdest = prec->bptr + start;
+        *pdest = *psource++;
+    }
+    nuse += n;
+    if(nuse > nsam) nuse = nsam;
+    offset = start + nuse;
+    if(offset>=nsam) offset -= nsam;
+
+    prec->off = offset;
+    prec->nuse = nuse;
+
+    return;
+}
+
+static void put_value(compressRecord *prec, double *psource, int n)
+{
+    switch(prec->balg) {
+       case bufferingALG_FIFO: put_value_FIFO(prec, psource, n); break;
+       case bufferingALG_LIFO: put_value_LIFO(prec, psource, n); break;
+    }
+    return;
+}
+
 

 /* qsort comparison function (for median calculation) */
 static int compare(const void *arg1, const void *arg2)
@@ -305,6 +343,7 @@
         prec->bptr = calloc(prec->nsam, sizeof(double));
         reset(prec);
     }
+
     return 0;
 }
 
@@ -388,18 +427,21 @@
     paddr->field_type = DBF_DOUBLE;
     paddr->field_size = sizeof(double);
     paddr->dbr_field_type = DBF_DOUBLE;
+
+    if(prec->balg == bufferingALG_LIFO) paddr->special = SPC_NOMOD;
     return 0;
 }
 
 static long get_array_info(DBADDR *paddr, long *no_elements, long *offset)
 {
     compressRecord *prec = (compressRecord *) paddr->precord;
+    epicsInt32 start = prec->off - prec->nuse;
+    if(start<0) start += prec->nsam;
+    if(prec->balg == bufferingALG_FIFO)
+         *no_elements = prec->nuse;
+    else *no_elements = prec->nsam;
+    *offset = start;
 
-    *no_elements = prec->nuse;
-    if (prec->nuse == prec->nsam)
-        *offset = prec->off;
-    else
-        *offset = 0;
     return 0;
 }
 

=== modified file 'src/std/rec/compressRecord.dbd.pod'
--- src/std/rec/compressRecord.dbd.pod	2013-09-30 22:48:06 +0000
+++ src/std/rec/compressRecord.dbd.pod	2015-07-09 21:24:24 +0000
@@ -41,6 +41,10 @@
 	choice(compressALG_Circular_Buffer,"Circular Buffer")
 	choice(compressALG_N_to_1_Median,"N to 1 Median")
 }
+menu(bufferingALG) {
+	choice(bufferingALG_FIFO, "FIFO Buffer")
+	choice(bufferingALG_LIFO, "LIFO Buffer")
+}
 recordtype(compress) {
 
 =fields VAL
@@ -76,6 +80,13 @@
 		interest(1)
 		menu(compressALG)
 	}
+ 	field(BALG,DBF_MENU) {
+		prompt("Buffering Algorithm")
+		promptgroup(GUI_ALARMS)
+		special(SPC_RESET)
+		interest(1)
+		menu(bufferingALG)
+	}
 	field(NSAM,DBF_ULONG) {
 		prompt("Number of Values")
 		promptgroup(GUI_COMPRESS)