Merge ~bfrk/epics-base:remove-dbfl_type_rec into ~epics-core/epics-base/+git/epics-base:7.0

[Ben Franksen]

This is just refactors, no new features yet.

[Ben Franksen (bfrk)]

I see now that the diff is dominated by my first commit that reformats dbAccess.c. The actual refactor makes only very few changes there. If this hurts I can rebase this patch out of the current branch. How do you people treat code reformatting patches? I did this one with my standard C indent config that very closely matches the standard EPICS base style.

[Ben Franksen (bfrk)]

I rebased and resubmitted to get rid of the reformat commit. Will create a different merge request for that one.

[Dirk Zimoch (dirk.zimoch)]

Apropos reformat commit: Should we do a general reformat of EPICS base? For example get rid of all the tabs.

[Ben Franksen (bfrk)]

> Apropos reformat commit: Should we do a general reformat of EPICS base? For
> example get rid of all the tabs.

I suggest to move this discussion to https://code.launchpad.net/~bfrk/epics-base/+git/epics-base/+merge/381153 or core-talk.

[Ben Franksen (bfrk)]

I have renamed my branch back to write-filters and removed the latest commit.

[Ben Franksen (bfrk)]

Okay, fresh and clean. Fixed the problem Ralph pointed out (in https://code.launchpad.net/~bfrk/epics-base/+git/epics-base/+merge/381308), rebased everything and resubmitted. Also renamed the branch because this has nothing (yet) to do with write/put filters.

[Andrew Johnson (anj)]

I haven't tried building or using this, but overall it looks reasonable. Unfortunately I don't think we can afford to change the prototype of dbGetField() like this because it is a published API that other modules call, but AFAIK they only ever pass NULL for the final argument – see my diff comment below for suggestions.

[Ben Franksen (bfrk)]

Am 31.03.20 um 22:17 schrieb Andrew Johnson:
> I haven't tried building or using this, but overall it looks reasonable. Unfortunately I don't think we can afford to change the prototype of dbGetField() like this because it is a published API that other modules call, but AFAIK they only ever pass NULL for the final argument – see my diff comment below for suggestions.

I offered this particular patch (commit 7287a1f) as kind of "for
discussion", as I wasn't sure how much external code still uses this
API. I only saw that the record types and device supports in base no
longer call dbGetField directly. BTW, the void *pflin member must have
been added at some point in time (it wasn't there in 3.14) and that must
have taken its toll on external modules, too. But that was a "major"
version jump so perhaps the rules allowed that.

Anyway, I can easily remove this particular patch. It is pretty much
independent from anything else I did or plan to do.

On another note, my current (unfinished) attempts at adding put filters
also include the addition of a new parameter to dbPut and dbPutField.
This is a similarly breaking change, and I guess I should do something
about that before I propose that code for merging, right?

>> diff --git a/modules/database/src/ioc/db/db_field_log.h b/modules/database/src/ioc/db/db_field_log.h
>> index 1534517..a801142 100644
>> --- a/modules/database/src/ioc/db/db_field_log.h
>> +++ b/modules/database/src/ioc/db/db_field_log.h
>> @@ -81,6 +92,8 @@ struct dbfl_val {
>> * db_delete_field_log(). Any code which changes a dbfl_type_ref
>> * field log to another type, or to reference different data,
>> * must explicitly call the dtor function.
>> + * If the dtor is NULL, then this means the array data is still owned
>> + * by a record.
>
> Would it make sense to provide a macro that evaluates that condition? The code I saw earlier that I assume was checking this looked somewhat scary for after a clean-up.

I guess you mean e.g. in dbAccess.c:

+ if ((!pfl || (pfl->type==dbfl_type_ref && !pfl->u.r.dtor)) &&
+ paddr->pfldDes->special == SPC_DBADDR &&
+ (prset = dbGetRset(paddr)) &&
+ prset->get_array_info) {
+ status = prset->get_array_info(paddr, &no_elements, &offset);
+ } else {
         offset = 0;
     }

Yes, it probably isn't too obvious to the reader what goes on here. In a
later patch, I have used a local boolean variable a la

+ int pfl_has_copy = pfl && (pfl->type != dbfl_type_ref ||
pfl->u.r.dtor);

but I guess offering (and using) a macro here is the right thing. Will fix.

>> +<<<<<<< modules/database/src/std/filters/arr.c
>
> I think the conflict markers shown here are really virtual, I believe they will disappear if/when we merge Dirk's branch that is marked as a prerequisite of this one.

The conflict is with a change you made on 7.0 since Dirk has created his
feature branch:

commit 25bb966cbc4be1f2a47116a8b9ec940bc2270b12
Author: Andrew Johnson <email address hidden>
Date: Sat Mar 14 16:19:26 2020 -0500

    Use the dbChannel*() accessor macros in the array filter code
    instead of exposing the dbChannel innards unnecessarily.

I wasn't sure how to set up my branch so...

85822f3... by Ben Franksen on 2020-04-01

add macro dbfl_has_copy to db_field_log.h and use it in dbAccess.c

It encapsulates the slightly tricky logic to decide whether a pointer
to a db_field_log has ownership of the data or not.

[Ben Franksen (bfrk)]

I removed the patch that changes the dbGetField API and added a new commit to address the point about "If the dtor is NULL, then this means the array data is still owned by a record".

The conflict is still there, pending a recommendation how to set up my branch properly.

[Ben Franksen (bfrk)]

Fixed a logical mistake in my last commit (rebased).

[Ben Franksen (bfrk)]

Since no-one came forward with a recommendation, I have now locally rebased all my changes such that they cleanly apply on branch 7.0. I did this by creating a new branch that forks off at the current HEAD of 7.0, then merging Dirk's branch dbChannelForDBLinks into that, and then replaying my changes, resolving the single conflict in the array filter implementation.

I have pushed this 'work' branch to my launchpad fork of epics base. All tests pass on all my intermediate versions.

Before I force-push the other branch refs (which are now on the work branch) to my fork (thereby updating all my merge requests), can somebody please tell me if this is what I should do?

[Andrew Johnson (anj)]

Core group 11/4: Will need some reworking after we have merged Dirk's other branch.

This will affect filters, breaking any external filter code (none known though).
The result looks cleaner so we like the idea in principle, but we can't merge it yet.

[Ben Franksen (bfrk)]

> The result looks cleaner so we like the idea in principle, but we can't merge
> it yet.

I can try to get this in shape (i.e. rebase it to the current HEAD). Are the prerequisites merged now? Should I move over to github (because of the automatic CI trigger there)?

[Andrew Johnson (anj)]

Hi Ben, yes I think everything else related has been merged now. Using GitHub instead of Launchpad does help with doing builds and running tests on our major architectures. Thanks!

[Ben Franksen (bfrk)]

See here: https://github.com/epics-base/epics-base/pull/99

Unmerged commits

85822f3... by Ben Franksen on 2020-04-01

add macro dbfl_has_copy to db_field_log.h and use it in dbAccess.c

It encapsulates the slightly tricky logic to decide whether a pointer
to a db_field_log has ownership of the data or not.

27fe3e4... by Ben Franksen on 2020-03-30

refactor db_field_log and filters to get rid of dbfl_type_rec

This refactor simplifies and streamlines the code associated with server
side filters. Apart from immediate benefits (clearer code, less duplication)
it is also hoped that this will make it easier to add write filters.

The data pointer dbfl_ref.field can now either point to a copy owned by a
filter, or it can point to the original data owned by a record. In the
latter case, the dbfl_ref.dtor is NULL.

The dbExtractArray* functions are unified to the single function
dbExtractArray and stripped of conversion functionality. This is redundant
because we always call dbGet after applying filters, which takes care of
conversion. Accordingly, dbChannelMakeArrayCopy is now obsolete and its
single use (in the ts filter) replaced with dbExtractArray. Instead, we add
the helper function dbChannelGetArrayInfo to wrap the common boilerplate
around calls to the get_array_info method, used in both arr.c and ts.c.

4ab9808... by Ben Franksen on 2020-03-30

make it clearer what the result of wrapArrayIndices will be

diff --git a/modules/database/src/ioc/db/dbAccess.c b/modules/database/src/ioc/db/dbAccess.c
index d7e5d08..3f7554a 100644
--- a/modules/database/src/ioc/db/dbAccess.c
+++ b/modules/database/src/ioc/db/dbAccess.c
@@ -339,7 +339,7 @@ static void getOptions(DBADDR *paddr, char **poriginal, long *options,
         dbCommon        *pcommon;
         char            *pbuffer = *poriginal;
 
-        if (!pfl || pfl->type == dbfl_type_rec)
+        if (!pfl)
             field_type = paddr->field_type;
         else
             field_type = pfl->field_type;
@@ -349,7 +349,7 @@ static void getOptions(DBADDR *paddr, char **poriginal, long *options,
         if( (*options) & DBR_STATUS ) {
             unsigned short *pushort = (unsigned short *)pbuffer;
 
-            if (!pfl || pfl->type == dbfl_type_rec) {
+            if (!pfl) {
                 *pushort++ = pcommon->stat;
                 *pushort++ = pcommon->sevr;
             } else {
@@ -383,7 +383,7 @@ static void getOptions(DBADDR *paddr, char **poriginal, long *options,
         if( (*options) & DBR_TIME ) {
             epicsUInt32 *ptime = (epicsUInt32 *)pbuffer;
 
-            if (!pfl || pfl->type == dbfl_type_rec) {
+            if (!pfl) {
                 *ptime++ = pcommon->time.secPastEpoch;
                 *ptime++ = pcommon->time.nsec;
             } else {
@@ -904,22 +904,23 @@ long dbGet(DBADDR *paddr, short dbrType,
     if (nRequest && *nRequest == 0)
         return 0;
 
-    if (!pfl || pfl->type == dbfl_type_rec) {
+    if (!pfl) {
         field_type = paddr->field_type;
         no_elements = capacity = paddr->no_elements;
-
-        /* Update field info from record
-         * may modify paddr->pfield
-         */
-        if (paddr->pfldDes->special == SPC_DBADDR &&
-            (prset = dbGetRset(paddr)) &&
-            prset->get_array_info) {
-            status = prset->get_array_info(paddr, &no_elements, &offset);
-        } else
-            offset = 0;
     } else {
         field_type = pfl->field_type;
         no_elements = capacity = pfl->no_elements;
+    }
+
+    /* Update field info from record (if neccessary);
+     * may modify paddr->pfield.
+     */
+    if (!dbfl_has_copy(pfl) &&
+        paddr->pfldDes->special == SPC_DBADDR &&
+        (prset = dbGetRset(paddr)) &&
+        prset->get_array_info) {
+        status = prset->get_array_info(paddr, &no_elements, &offset);
+    } else {
         offset = 0;
     }
 
@@ -951,7 +952,7 @@ long dbGet(DBADDR *paddr, short dbrType,
             goto done;
         }
 
-        if (!pfl || pfl->type == dbfl_type_rec) {
+        if (!pfl) {
             status = dbFastGetConvertRoutine[field_type][dbrType]
                 (paddr->pfield, pbuf, paddr);
         } else {
@@ -964,6 +965,7 @@ long dbGet(DBADDR *paddr, short dbrType,
 
             localAddr.field_type = pfl->field_type;
             localAddr.field_size = pfl->field_size;
+            /* not used by dbFastConvert: */
             localAddr.no_elements = pfl->no_elements;
             if (pfl->type == dbfl_type_val)
                 localAddr.pfield = (char *) &pfl->u.v.field;
@@ -979,6 +981,8 @@ long dbGet(DBADDR *paddr, short dbrType,
         if (nRequest) {
             if (no_elements < *nRequest)
                 *nRequest = no_elements;
+            if (capacity < *nRequest)
+                *nRequest = capacity;
             n = *nRequest;
         } else {
             n = 1;
@@ -995,8 +999,8 @@ long dbGet(DBADDR *paddr, short dbrType,
         }
         /* convert data into the caller's buffer */
         if (n <= 0) {
-            ;/*do nothing*/
-        } else if (!pfl || pfl->type == dbfl_type_rec) {
+            ;                           /*do nothing */
+        } else if (!pfl) {
             status = convert(paddr, pbuf, n, capacity, offset);
         } else {
             DBADDR localAddr = *paddr; /* Structure copy */
@@ -1008,6 +1012,7 @@ long dbGet(DBADDR *paddr, short dbrType,
 
             localAddr.field_type = pfl->field_type;
             localAddr.field_size = pfl->field_size;
+            /* not used by dbConvert, it uses the passed capacity instead: */
             localAddr.no_elements = pfl->no_elements;
             if (pfl->type == dbfl_type_val)
                 localAddr.pfield = (char *) &pfl->u.v.field;
diff --git a/modules/database/src/ioc/db/dbChannel.c b/modules/database/src/ioc/db/dbChannel.c
index b6f53f7..c71d842 100644
--- a/modules/database/src/ioc/db/dbChannel.c
+++ b/modules/database/src/ioc/db/dbChannel.c
@@ -52,14 +52,12 @@ typedef struct parseContext {
 
 static void *dbChannelFreeList;
 static void *chFilterFreeList;
-static void *dbchStringFreeList;
 
 void dbChannelExit(void)
 {
     freeListCleanup(dbChannelFreeList);
     freeListCleanup(chFilterFreeList);
-    freeListCleanup(dbchStringFreeList);
-    dbChannelFreeList = chFilterFreeList = dbchStringFreeList = NULL;
+    dbChannelFreeList = chFilterFreeList = NULL;
 }
 
 void dbChannelInit (void)
@@ -69,7 +67,6 @@ void dbChannelInit (void)
 
     freeListInitPvt(&dbChannelFreeList,  sizeof(dbChannel), 128);
     freeListInitPvt(&chFilterFreeList,  sizeof(chFilter), 64);
-    freeListInitPvt(&dbchStringFreeList, sizeof(epicsOldString), 128);
     db_init_event_freelists();
 }
 
@@ -449,28 +446,6 @@ static long parseArrayRange(dbChannel* chan, const char *pname, const char **ppn
     return status;
 }
 
-/* Stolen from dbAccess.c: */
-static short mapDBFToDBR[DBF_NTYPES] =
-    {
-    /* DBF_STRING   => */DBR_STRING,
-    /* DBF_CHAR     => */DBR_CHAR,
-    /* DBF_UCHAR    => */DBR_UCHAR,
-    /* DBF_SHORT    => */DBR_SHORT,
-    /* DBF_USHORT   => */DBR_USHORT,
-    /* DBF_LONG     => */DBR_LONG,
-    /* DBF_ULONG    => */DBR_ULONG,
-    /* DBF_INT64    => */DBR_INT64,
-    /* DBF_UINT64   => */DBR_UINT64,
-    /* DBF_FLOAT    => */DBR_FLOAT,
-    /* DBF_DOUBLE   => */DBR_DOUBLE,
-    /* DBF_ENUM,    => */DBR_ENUM,
-    /* DBF_MENU,    => */DBR_ENUM,
-    /* DBF_DEVICE   => */DBR_ENUM,
-    /* DBF_INLINK   => */DBR_STRING,
-    /* DBF_OUTLINK  => */DBR_STRING,
-    /* DBF_FWDLINK  => */DBR_STRING,
-    /* DBF_NOACCESS => */DBR_NOACCESS };
-
 dbChannel * dbChannelCreate(const char *name)
 {
     const char *pname = name;
@@ -743,37 +718,24 @@ void dbChannelDelete(dbChannel *chan)
     freeListFree(dbChannelFreeList, chan);
 }
 
-static void freeArray(db_field_log *pfl) {
-    if (pfl->field_type == DBF_STRING && pfl->no_elements == 1) {
-        freeListFree(dbchStringFreeList, pfl->u.r.field);
-    } else {
-        free(pfl->u.r.field);
-    }
-}
-
-void dbChannelMakeArrayCopy(void *pvt, db_field_log *pfl, dbChannel *chan)
+/*
+ * Helper function to adjust no_elements, offset, and pfield
+ * when copying an array from a record.
+ */
+void dbChannelGetArrayInfo(dbChannel *chan,
+    void **pfield, long *no_elements, long *offset)
 {
-    void *p;
-    struct dbCommon *prec = dbChannelRecord(chan);
-
-    if (pfl->type != dbfl_type_rec) return;
-
-    pfl->type = dbfl_type_ref;
-    pfl->stat = prec->stat;
-    pfl->sevr = prec->sevr;
-    pfl->time = prec->time;
-    pfl->field_type  = chan->addr.field_type;
-    pfl->no_elements = chan->addr.no_elements;
-    pfl->field_size  = chan->addr.field_size;
-    pfl->u.r.dtor = freeArray;
-    pfl->u.r.pvt = pvt;
-    if (pfl->field_type == DBF_STRING && pfl->no_elements == 1) {
-        p = freeListCalloc(dbchStringFreeList);
-    } else {
-        p = calloc(pfl->no_elements, pfl->field_size);
+    rset *prset;
+    if (dbChannelSpecial(chan) == SPC_DBADDR &&
+        (prset = dbGetRset(&chan->addr)) &&
+        prset->get_array_info)
+    {
+        void *pfieldsave = dbChannelField(chan);
+        /* it is expected that this call always succeeds */
+        prset->get_array_info(&chan->addr, no_elements, offset);
+        *pfield = dbChannelField(chan);
+        dbChannelField(chan) = pfieldsave;
     }
-    if (p) dbGet(&chan->addr, mapDBFToDBR[pfl->field_type], p, NULL, &pfl->no_elements, NULL);
-    pfl->u.r.field = p;
 }
 
 /* FIXME: Do these belong in a different file? */
diff --git a/modules/database/src/ioc/db/dbChannel.h b/modules/database/src/ioc/db/dbChannel.h
index 1ca02bb..ec86e9e 100644
--- a/modules/database/src/ioc/db/dbChannel.h
+++ b/modules/database/src/ioc/db/dbChannel.h
@@ -65,8 +65,8 @@ typedef struct dbChannel {
 /* Prototype for the channel event function that is called in filter stacks
  *
  * When invoked the scan lock for the record associated with 'chan' _may_ be locked.
- * If pLog->type==dbfl_type_rec then dbScanLock() must be called before copying
- * data out of the associated record.
+ * Unless dbfl_has_copy(pLog), it must call dbScanLock before accessing the data,
+ * as this indicates the data is still owned by the record.
  *
  * This function has ownership of the field log pLog, if it wishes to discard
  * this update it should free the field log with db_delete_field_log() and
@@ -225,7 +225,8 @@ DBCORE_API void dbRegisterFilter(const char *key, const chFilterIf *fif, void *p
 DBCORE_API db_field_log* dbChannelRunPreChain(dbChannel *chan, db_field_log *pLogIn);
 DBCORE_API db_field_log* dbChannelRunPostChain(dbChannel *chan, db_field_log *pLogIn);
 DBCORE_API const chFilterPlugin * dbFindFilter(const char *key, size_t len);
-DBCORE_API void dbChannelMakeArrayCopy(void *pvt, db_field_log *pfl, dbChannel *chan);
+DBCORE_API void dbChannelGetArrayInfo(dbChannel *chan,
+        void **pfield, long *no_elements, long *offset);
 
 #ifdef __cplusplus
 }
diff --git a/modules/database/src/ioc/db/dbEvent.c b/modules/database/src/ioc/db/dbEvent.c
index 437cb6d..1db01f2 100644
--- a/modules/database/src/ioc/db/dbEvent.c
+++ b/modules/database/src/ioc/db/dbEvent.c
@@ -668,27 +668,21 @@ int db_post_extra_labor (dbEventCtx ctx)
     return DB_EVENT_OK;
 }
 
-/*
- *  DB_CREATE_EVENT_LOG()
- *
- *  NOTE: This assumes that the db scan lock is already applied
- *        (as it copies data from the record)
- */
-db_field_log* db_create_event_log (struct evSubscrip *pevent)
+static db_field_log* db_create_field_log (struct dbChannel *chan, int use_val)
 {
     db_field_log *pLog = (db_field_log *) freeListCalloc(dbevFieldLogFreeList);
 
     if (pLog) {
-        struct dbChannel *chan = pevent->chan;
         struct dbCommon  *prec = dbChannelRecord(chan);
-        pLog->ctx = dbfl_context_event;
-        if (pevent->useValque) {
+        pLog->stat = prec->stat;
+        pLog->sevr = prec->sevr;
+        pLog->time = prec->time;
+        pLog->field_type  = dbChannelFieldType(chan);
+        pLog->field_size  = dbChannelFieldSize(chan);
+        pLog->no_elements = dbChannelElements(chan);
+
+        if (use_val) {
             pLog->type = dbfl_type_val;
-            pLog->stat = prec->stat;
-            pLog->sevr = prec->sevr;
-            pLog->time = prec->time;
-            pLog->field_type  = dbChannelFieldType(chan);
-            pLog->no_elements = dbChannelElements(chan);
             /*
              * use memcpy to avoid a bus error on
              * union copy of char in the db at an odd
@@ -698,23 +692,46 @@ db_field_log* db_create_event_log (struct evSubscrip *pevent)
                    dbChannelField(chan),
                    dbChannelFieldSize(chan));
         } else {
-            pLog->type = dbfl_type_rec;
+            pLog->type = dbfl_type_ref;
+
+            /* don't make a copy yet, just reference the field value */
+            pLog->u.r.field = dbChannelField(chan);
+            /* indicate field value still owned by record */
+            pLog->u.r.dtor = NULL;
+            /* no private data yet, may be set by a filter */
+            pLog->u.r.pvt = NULL;
         }
     }
     return pLog;
 }
 
 /*
+ *  DB_CREATE_EVENT_LOG()
+ *
+ *  NOTE: This assumes that the db scan lock is already applied
+ *        (as it calls rset->get_array_info)
+ */
+db_field_log* db_create_event_log (struct evSubscrip *pevent)
+{
+    db_field_log *pLog = db_create_field_log(pevent->chan, pevent->useValque);
+    if (pLog) {
+        pLog->ctx  = dbfl_context_event;
+    }
+    return pLog;
+}
+
+/*
  *  DB_CREATE_READ_LOG()
  *
  */
 db_field_log* db_create_read_log (struct dbChannel *chan)
 {
-    db_field_log *pLog = (db_field_log *) freeListCalloc(dbevFieldLogFreeList);
-
+    db_field_log *pLog = db_create_field_log(chan,
+        dbChannelElements(chan) == 1 &&
+        dbChannelSpecial(chan) != SPC_DBADDR &&
+        dbChannelFieldSize(chan) <= sizeof(union native_value));
     if (pLog) {
         pLog->ctx  = dbfl_context_read;
-        pLog->type = dbfl_type_rec;
     }
     return pLog;
 }
@@ -738,20 +755,6 @@ static void db_queue_event_log (evSubscrip *pevent, db_field_log *pLog)
     LOCKEVQUE (ev_que);
 
     /*
-     * if we have an event on the queue and both the last
-     * event on the queue and the current event are emtpy
-     * (i.e. of type dbfl_type_rec), simply ignore duplicate
-     * events (saving empty events serves no purpose)
-     */
-    if (pevent->npend > 0u &&
-        (*pevent->pLastLog)->type == dbfl_type_rec &&
-        pLog->type == dbfl_type_rec) {
-        db_delete_field_log(pLog);
-        UNLOCKEVQUE (ev_que);
-        return;
-    }
-
-    /*
      * add to task local event que
      */
 
diff --git a/modules/database/src/ioc/db/dbExtractArray.c b/modules/database/src/ioc/db/dbExtractArray.c
index a7dcf4d..197e66c 100644
--- a/modules/database/src/ioc/db/dbExtractArray.c
+++ b/modules/database/src/ioc/db/dbExtractArray.c
@@ -14,11 +14,12 @@
 /*
  *  Author: Ralph Lange <[e-mail]>
  *
- *  based on dbConvert.c
+ *  based on dbConvert.c, see copyNoConvert
  *  written by: Bob Dalesio, Marty Kraimer
  */
 
 #include <string.h>
+#include <assert.h>
 
 #include "epicsTypes.h"
 
@@ -26,61 +27,30 @@
 #include "dbAddr.h"
 #include "dbExtractArray.h"
 
-void dbExtractArrayFromRec(const dbAddr *paddr, void *pto,
-                           long nRequest, long no_elements, long offset, long increment)
+void dbExtractArray(const void *pfrom, void *pto, short field_size,
+    long nRequest, long no_elements, long offset, long increment)
 {
     char *pdst = (char *) pto;
-    char *psrc = (char *) paddr->pfield;
-    long nUpperPart;
-    int i;
-    short srcSize = paddr->field_size;
-    short dstSize = srcSize;
-    char isString = (paddr->field_type == DBF_STRING);
+    const char *psrc = (char *) pfrom;
 
-    if (nRequest > no_elements) nRequest = no_elements;
-    if (isString && srcSize > MAX_STRING_SIZE) dstSize = MAX_STRING_SIZE;
+    /* assert preconditions */
+    assert(nRequest >= 0);
+    assert(no_elements >= 0);
+    assert(increment > 0);
+    assert(0 <= offset);
+    assert(offset < no_elements);
 
-    if (increment == 1 && dstSize == srcSize) {
-        nUpperPart = nRequest < no_elements - offset ? nRequest : no_elements - offset;
-        memcpy(pdst, &psrc[offset * srcSize], dstSize * nUpperPart);
+    if (increment == 1) {
+        long nUpperPart =
+            nRequest < no_elements - offset ? nRequest : no_elements - offset;
+        memcpy(pdst, psrc + (offset * field_size), field_size * nUpperPart);
         if (nRequest > nUpperPart)
-            memcpy(&pdst[dstSize * nUpperPart], psrc, dstSize * (nRequest - nUpperPart));
-        if (isString)
-            for (i = 1; i <= nRequest; i++)
-                pdst[dstSize*i-1] = '\0';
+            memcpy(pdst + (field_size * nUpperPart), psrc,
+                field_size * (nRequest - nUpperPart));
     } else {
-        for (; nRequest > 0; nRequest--, pdst += dstSize, offset += increment) {
+        for (; nRequest > 0; nRequest--, pdst += field_size, offset += increment) {
             offset %= no_elements;
-            memcpy(pdst, &psrc[offset*srcSize], dstSize);
-            if (isString) pdst[dstSize-1] = '\0';
-        }
-    }
-}
-
-void dbExtractArrayFromBuf(const void *pfrom, void *pto,
-                           short field_size, short field_type,
-                           long nRequest, long no_elements, long offset, long increment)
-{
-    char *pdst = (char *) pto;
-    char *psrc = (char *) pfrom;
-    int i;
-    short srcSize = field_size;
-    short dstSize = srcSize;
-    char isString = (field_type == DBF_STRING);
-
-    if (nRequest > no_elements) nRequest = no_elements;
-    if (offset > no_elements - 1) offset = no_elements - 1;
-    if (isString && dstSize >= MAX_STRING_SIZE) dstSize = MAX_STRING_SIZE - 1;
-
-    if (increment == 1) {
-        memcpy(pdst, &psrc[offset * srcSize], dstSize * nRequest);
-        if (isString)
-            for (i = 1; i <= nRequest; i++)
-                pdst[dstSize*i] = '\0';
-    } else {
-        for (; nRequest > 0; nRequest--, pdst += srcSize, offset += increment) {
-            memcpy(pdst, &psrc[offset*srcSize], dstSize);
-            if (isString) pdst[dstSize] = '\0';
+            memcpy(pdst, psrc + (offset * field_size), field_size);
         }
     }
 }
diff --git a/modules/database/src/ioc/db/dbExtractArray.h b/modules/database/src/ioc/db/dbExtractArray.h
index b44bd15..9755ac5 100644
--- a/modules/database/src/ioc/db/dbExtractArray.h
+++ b/modules/database/src/ioc/db/dbExtractArray.h
@@ -22,11 +22,36 @@
 extern "C" {
 #endif
 
-epicsShareFunc void dbExtractArrayFromRec(const DBADDR *paddr, void *pto,
-                                          long nRequest, long no_elements, long offset, long increment);
-epicsShareFunc void dbExtractArrayFromBuf(const void *pfrom, void *pto,
-                                          short field_size, short field_type,
-                                          long nRequest, long no_elements, long offset, long increment);
+/** @brief Make a copy of parts of an array.
+ *
+ * The source array may or may not be a record field.
+ *
+ * The increment parameter is used to support array filters; it
+ * means: copy only every increment'th element, starting at offset.
+ *
+ * The offset and no_elements parameters are used to support the
+ * circular buffer feature of record fields: elements before offset
+ * are treated as if they came right after no_elements.
+ *
+ * This function does not do any conversion on the array elements.
+ *
+ * Preconditions:
+ *   nRequest >= 0, no_elements >= 0, increment > 0
+ *   0 <= offset < no_elements
+ *   pto points to a buffer with at least field_size*nRequest bytes
+ *   pfrom points to a buffer with exactly field_size*no_elements bytes
+ *
+ * @param pfrom         Pointer to source array.
+ * @param pto           Pointer to target array.
+ * @param field_size    Size of an array element.
+ * @param nRequest      Number of elements to copy.
+ * @param no_elements   Number of elements in source array.
+ * @param offset        Wrap-around point in source array.
+ * @param increment     Copy only every increment'th element.
+ */
+epicsShareFunc void dbExtractArray(const void *pfrom, void *pto,
+    short field_size, long nRequest, long no_elements, long offset,
+    long increment);
 
 #ifdef __cplusplus
 }
diff --git a/modules/database/src/ioc/db/db_field_log.h b/modules/database/src/ioc/db/db_field_log.h
index 5a40f5f..222bad0 100644
--- a/modules/database/src/ioc/db/db_field_log.h
+++ b/modules/database/src/ioc/db/db_field_log.h
@@ -57,20 +57,31 @@ union native_value {
 struct db_field_log;
 typedef void (dbfl_freeFunc)(struct db_field_log *pfl);
 
-/* Types of db_field_log: rec = use record, val = val inside, ref = reference inside */
+/*
+ * A db_field_log has one of two types:
+ *
+ * dbfl_type_ref - Reference to value
+ *  Used for variable size (array) data types.  Meta-data
+ *  is stored in the field log, but value data is stored externally.
+ *  Only the dbfl_ref side of the data union is valid.
+ *
+ * dbfl_type_val - Internal value
+ *  Used to store small scalar data.  Meta-data and value are
+ *  present in this structure and no external references are used.
+ *  Only the dbfl_val side of the data union is valid.
+ */
 typedef enum dbfl_type {
-    dbfl_type_rec = 0,
     dbfl_type_val,
     dbfl_type_ref
 } dbfl_type;
 
 /* Context of db_field_log: event = subscription update, read = read reply */
 typedef enum dbfl_context {
-    dbfl_context_read = 0,
+    dbfl_context_read,
     dbfl_context_event
 } dbfl_context;
 
-#define dbflTypeStr(t) (t==dbfl_type_val?"val":t==dbfl_type_rec?"rec":"ref")
+#define dbflTypeStr(t) (t==dbfl_type_val?"val":"ref")
 
 struct dbfl_val {
     union native_value field; /* Field value */
@@ -82,6 +93,8 @@ struct dbfl_val {
  * db_delete_field_log().  Any code which changes a dbfl_type_ref
  * field log to another type, or to reference different data,
  * must explicitly call the dtor function.
+ * If the dtor is NULL and no_elements > 0, then this means the array
+ * data is still owned by a record. See the macro dbfl_has_copy below.
  */
 struct dbfl_ref {
     dbfl_freeFunc     *dtor;  /* Callback to free filter-allocated resources */
@@ -89,8 +102,11 @@ struct dbfl_ref {
     void              *field; /* Field value */
 };
 
+/*
+ * Note that field_size may be larger than MAX_STRING_SIZE.
+ */
 typedef struct db_field_log {
-    unsigned int     type:2;  /* type (union) selector */
+    unsigned int     type:1;  /* type (union) selector */
     /* ctx is used for all types */
     unsigned int      ctx:1;  /* context (operation type) */
     /* the following are used for value and reference types */
@@ -98,8 +114,8 @@ typedef struct db_field_log {
     unsigned short     stat;  /* Alarm Status */
     unsigned short     sevr;  /* Alarm Severity */
     short        field_type;  /* DBF type of data */
-    short        field_size;  /* Data size */
-    long        no_elements;  /* No of array elements */
+    short        field_size;  /* Size of a single element */
+    long        no_elements;  /* No of valid array elements */
     union {
         struct dbfl_val v;
         struct dbfl_ref r;
@@ -107,27 +123,16 @@ typedef struct db_field_log {
 } db_field_log;
 
 /*
- * A db_field_log will in one of three types:
- *
- * dbfl_type_rec - Reference to record
- *  The field log stores no data itself.  Data must instead be taken
- *  via the dbChannel* which must always be provided when along
- *  with the field log.
- *  For this type only the 'type' and 'ctx' members are used.
- *
- * dbfl_type_ref - Reference to outside value
- *  Used for variable size (array) data types.  Meta-data
- *  is stored in the field log, but value data is stored externally
- *  (see struct dbfl_ref).
- *  For this type all meta-data members are used.  The dbfl_ref side of the
- *  data union is used.
- *
- * dbfl_type_val - Internal value
- *  Used to store small scalar data.  Meta-data and value are
- *  present in this structure and no external references are used.
- *  For this type all meta-data members are used.  The dbfl_val side of the
- *  data union is used.
+ * Whether a db_field_log* owns the field data. If this is the case, then the
+ * db_field_log is fully decoupled from the record: there is no need to lock
+ * the record when accessing the data, nor to call any rset methods (like
+ * get_array_info) because this has already been done when we made the copy. A
+ * special case here is that of no (remaining) data (i.e. no_elements==0). In
+ * this case, making a copy is redundant, so we have no dtor. But conceptually
+ * the db_field_log still owns the (empty) data.
  */
+#define dbfl_has_copy(p)\
+ ((p) && ((p)->type==dbfl_type_val || (p)->u.r.dtor || (p)->no_elements==0))
 
 #ifdef __cplusplus
 }
diff --git a/modules/database/src/std/filters/arr.c b/modules/database/src/std/filters/arr.c
index 8138480..5f25184 100644
--- a/modules/database/src/std/filters/arr.c
+++ b/modules/database/src/std/filters/arr.c
@@ -13,16 +13,14 @@
 
 #include <stdio.h>
 
-#include <freeList.h>
-#include <dbAccess.h>
-#include <dbExtractArray.h>
-#include <db_field_log.h>
-#include <dbLock.h>
-#include <recSup.h>
-#include <epicsExit.h>
-#include <special.h>
-#include <chfPlugin.h>
-#include <epicsExport.h>
+#include "chfPlugin.h"
+#include "dbAccessDefs.h"
+#include "dbExtractArray.h"
+#include "db_field_log.h"
+#include "dbLock.h"
+#include "epicsExit.h"
+#include "freeList.h"
+#include "epicsExport.h"
 
 typedef struct myStruct {
     epicsInt32 start;
@@ -46,6 +44,8 @@ static void * allocPvt(void)
     myStruct *my = (myStruct*) freeListCalloc(myStructFreeList);
     if (!my) return NULL;
 
+    /* defaults */
+    my->start = 0;
     my->incr = 1;
     my->end = -1;
     return (void *) my;
@@ -77,8 +77,6 @@ static void freeArray(db_field_log *pfl)
 static long wrapArrayIndices(long *start, const long increment, long *end,
     const long no_elements)
 {
-    long len = 0;
-
     if (*start < 0) *start = no_elements + *start;
     if (*start < 0) *start = 0;
     if (*start > no_elements) *start = no_elements;
@@ -87,85 +85,65 @@ static long wrapArrayIndices(long *start, const long increment, long *end,
     if (*end < 0) *end = 0;
     if (*end >= no_elements) *end = no_elements - 1;
 
-    if (*end - *start >= 0) len = 1 + (*end - *start) / increment;
-    return len;
+    if (*end - *start >= 0)
+        return 1 + (*end - *start) / increment;
+    else
+        return 0;
 }
 
 static db_field_log* filter(void* pvt, dbChannel *chan, db_field_log *pfl)
 {
     myStruct *my = (myStruct*) pvt;
-    struct dbCommon *prec;
-    rset *prset;
+    int must_lock;
     long start = my->start;
     long end = my->end;
-    long nTarget = 0;
+    long nTarget;
+    void *pTarget;
     long offset = 0;
-    long nSource = dbChannelElements(chan);
-    long capacity = nSource;
-    void *pdst;
+    long nSource = pfl->no_elements;
+    void *pSource = pfl->u.r.field;
 
     switch (pfl->type) {
     case dbfl_type_val:
-        /* Only filter arrays */
-        break;
-
-    case dbfl_type_rec:
-        /* Extract from record */
-        if (dbChannelSpecial(chan) == SPC_DBADDR &&
-            nSource > 1 &&
-            (prset = dbGetRset(&chan->addr)) &&
-            prset->get_array_info)
-        {
-            void *pfieldsave = dbChannelField(chan);
-            prec = dbChannelRecord(chan);
-            dbScanLock(prec);
-            prset->get_array_info(&chan->addr, &nSource, &offset);
-            nTarget = wrapArrayIndices(&start, my->incr, &end, nSource);
-            pfl->type = dbfl_type_ref;
-            pfl->stat = prec->stat;
-            pfl->sevr = prec->sevr;
-            pfl->time = prec->time;
-            pfl->field_type = dbChannelFieldType(chan);
-            pfl->field_size = dbChannelFieldSize(chan);
-            pfl->no_elements = nTarget;
-            if (nTarget) {
-                pdst = freeListCalloc(my->arrayFreeList);
-                if (pdst) {
-                    pfl->u.r.dtor = freeArray;
-                    pfl->u.r.pvt = my->arrayFreeList;
-                    offset = (offset + start) % dbChannelElements(chan);
-                    dbExtractArrayFromRec(&chan->addr, pdst, nTarget, capacity,
-                        offset, my->incr);
-                    pfl->u.r.field = pdst;
-                }
-            }
-            dbScanUnlock(prec);
-            dbChannelField(chan) = pfieldsave;
-        }
+        /* TODO Treat scalars as arrays with 1 element */
         break;
 
-    /* Extract from buffer */
     case dbfl_type_ref:
-        pdst = NULL;
-        nSource = pfl->no_elements;
-        nTarget = wrapArrayIndices(&start, my->incr, &end, nSource);
-        pfl->no_elements = nTarget;
-        if (nTarget) {
-            /* Copy the data out */
-            void *psrc = pfl->u.r.field;
-
-            pdst = freeListCalloc(my->arrayFreeList);
-            if (!pdst) break;
-            offset = start;
-            dbExtractArrayFromBuf(psrc, pdst, pfl->field_size, pfl->field_type,
-                nTarget, nSource, offset, my->incr);
+        must_lock = !pfl->u.r.dtor;
+        if (must_lock) {
+            dbScanLock(dbChannelRecord(chan));
+            dbChannelGetArrayInfo(chan, &pSource, &nSource, &offset);
         }
-        if (pfl->u.r.dtor) pfl->u.r.dtor(pfl);
-        if (nTarget) {
+        nTarget = wrapArrayIndices(&start, my->incr, &end, nSource);
+        if (nTarget > 0) {
+            /* copy the data */
+            pTarget = freeListCalloc(my->arrayFreeList);
+            if (!pTarget) break;
+            /* must do the wrap-around with the original no_elements */
+            offset = (offset + start) % pfl->no_elements;
+            dbExtractArray(pSource, pTarget, pfl->field_size,
+                nTarget, pfl->no_elements, offset, my->incr);
+            if (pfl->u.r.dtor) pfl->u.r.dtor(pfl);
+            pfl->u.r.field = pTarget;
             pfl->u.r.dtor = freeArray;
             pfl->u.r.pvt = my->arrayFreeList;
-            pfl->u.r.field = pdst;
         }
+        /* Adjust no_elements to refer to the new pTarget.
+         *
+         * Setting pfl->no_elements outside of the "if" clause above is
+         * done to make requests fail if nTarget is zero, that is, if all
+         * elements selected by the filter are outside the array bounds.
+         * TODO:
+         * It would be possible to lift this restriction by interpreting
+         * a request with *no* number of elements (NULL pointer) as scalar
+         * (meaning: fail if we get less than one element); in contrast,
+         * a request that explicitly specifies one element would be
+         * interpreted as an array request, for which zero elements would
+         * be a normal expected result.
+         */
+        pfl->no_elements = nTarget;
+        if (must_lock)
+            dbScanUnlock(dbChannelRecord(chan));
         break;
     }
     return pfl;
diff --git a/modules/database/src/std/filters/ts.c b/modules/database/src/std/filters/ts.c
index 8a07c3f..0feadb0 100644
--- a/modules/database/src/std/filters/ts.c
+++ b/modules/database/src/std/filters/ts.c
@@ -12,21 +12,44 @@
  */
 
 #include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
 
-#include <chfPlugin.h>
-#include <dbLock.h>
-#include <db_field_log.h>
-#include <epicsExport.h>
+#include "chfPlugin.h"
+#include "db_field_log.h"
+#include "dbExtractArray.h"
+#include "dbLock.h"
+#include "epicsExport.h"
+
+/*
+ * The size of the data is different for each channel, and can even
+ * change at runtime, so a freeList doesn't make much sense here.
+ */
+static void freeArray(db_field_log *pfl) {
+    free(pfl->u.r.field);
+}
 
 static db_field_log* filter(void* pvt, dbChannel *chan, db_field_log *pfl) {
     epicsTimeStamp now;
     epicsTimeGetCurrent(&now);
 
-    /* If string or array, must make a copy (to ensure coherence between time and data) */
-    if (pfl->type == dbfl_type_rec) {
-        dbScanLock(dbChannelRecord(chan));
-        dbChannelMakeArrayCopy(pvt, pfl, chan);
-        dbScanUnlock(dbChannelRecord(chan));
+    /* If reference and not already copied,
+       must make a copy (to ensure coherence between time and data) */
+    if (pfl->type == dbfl_type_ref && !pfl->u.r.dtor) {
+        void *pTarget = calloc(pfl->no_elements, pfl->field_size);
+        void *pSource = pfl->u.r.field;
+        if (pTarget) {
+            long offset = 0;
+            long nSource = pfl->no_elements;
+            dbScanLock(dbChannelRecord(chan));
+            dbChannelGetArrayInfo(chan, &pSource, &nSource, &offset);
+            dbExtractArray(pSource, pTarget, pfl->field_size,
+                nSource, pfl->no_elements, offset, 1);
+            pfl->u.r.field = pTarget;
+            pfl->u.r.dtor = freeArray;
+            pfl->u.r.pvt = pvt;
+            dbScanUnlock(dbChannelRecord(chan));
+        }
     }
 
     pfl->time = now;
diff --git a/modules/database/test/ioc/db/dbChArrTest.cpp b/modules/database/test/ioc/db/dbChArrTest.cpp
index 90702b5..9965852 100644
--- a/modules/database/test/ioc/db/dbChArrTest.cpp
+++ b/modules/database/test/ioc/db/dbChArrTest.cpp
@@ -131,7 +131,7 @@ static void check(short dbr_type) {
     memset(buf, 0, sizeof(buf)); \
     (void) dbPutField(&offaddr, DBR_LONG, &off, 1); \
     pfl = db_create_read_log(pch); \
-    testOk(pfl && pfl->type == dbfl_type_rec, "Valid pfl, type = rec"); \
+    testOk(pfl && pfl->type == dbfl_type_ref, "Valid pfl, type = ref"); \
     testOk(!dbChannelGetField(pch, DBR_LONG, buf, NULL, &req, pfl), "Got Field value"); \
     testOk(req == Size, "Got %ld elements (expected %d)", req, Size); \
     if (!testOk(!memcmp(buf, Expected, sizeof(Expected)), "Data correct")) \
diff --git a/modules/database/test/std/filters/arrTest.cpp b/modules/database/test/std/filters/arrTest.cpp
index bd83bd8..dfbbf46 100644
--- a/modules/database/test/std/filters/arrTest.cpp
+++ b/modules/database/test/std/filters/arrTest.cpp
@@ -73,9 +73,9 @@ static int fl_equals_array(short type, const db_field_log *pfl1, void *p2) {
             }
             break;
         case DBR_STRING:
-            if (strtol(&((const char*)pfl1->u.r.field)[i*MAX_STRING_SIZE], NULL, 0) != ((epicsInt32*)p2)[i]) {
+            if (strtol(&((const char*)pfl1->u.r.field)[i*pfl1->field_size], NULL, 0) != ((epicsInt32*)p2)[i]) {
                 testDiag("at index=%d: field log has '%s', should be '%d'",
-                         i, &((const char*)pfl1->u.r.field)[i*MAX_STRING_SIZE], ((epicsInt32*)p2)[i]);
+                         i, &((const char*)pfl1->u.r.field)[i*pfl1->field_size], ((epicsInt32*)p2)[i]);
                 return 0;
             }
             break;
@@ -120,7 +120,7 @@ static void testHead (const char *title, const char *typ = "") {
     off = Offset; \
     (void) dbPutField(&offaddr, DBR_LONG, &off, 1); \
     pfl = db_create_read_log(pch); \
-    testOk(pfl->type == dbfl_type_rec, "original field log has type rec"); \
+    testOk(pfl->type == dbfl_type_ref, "original field log has type ref"); \
     pfl2 = dbChannelRunPostChain(pch, pfl); \
     testOk(pfl2 == pfl, "call does not drop or replace field_log"); \
     testOk(pfl->type == dbfl_type_ref, "filtered field log has type ref"); \
diff --git a/modules/database/test/std/filters/dbndTest.c b/modules/database/test/std/filters/dbndTest.c
index e7897e2..0206865 100644
--- a/modules/database/test/std/filters/dbndTest.c
+++ b/modules/database/test/std/filters/dbndTest.c
@@ -130,7 +130,7 @@ MAIN(dbndTest)
     dbEventCtx evtctx;
     int logsFree, logsFinal;
 
-    testPlan(77);
+    testPlan(72);
 
     testdbPrepare();
 
@@ -171,12 +171,9 @@ MAIN(dbndTest)
            "dbnd has one filter with argument in pre chain");
     testOk((ellCount(&pch->post_chain) == 0), "dbnd has no filter in post chain");
 
-    /* Field logs of type ref and rec: pass any update */
-
-    testHead("Field logs of type ref and rec");
-    fl1.type = dbfl_type_rec;
-    mustPassTwice(pch, &fl1, "abs field_log=rec", 0., 0);
+    /* Field logs of type ref: pass any update */
 
+    testHead("Field logs of type ref");
     fl1.type = dbfl_type_ref;
     mustPassTwice(pch, &fl1, "abs field_log=ref", 0., 0);