From 27fe3e4468ec62d4d17f775c4aced939a666ba36 Mon Sep 17 00:00:00 2001 From: Ben Franksen Date: Mon, 30 Mar 2020 21:34:32 +0200 Subject: [PATCH] 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. --- modules/database/src/ioc/db/dbAccess.c | 39 +++--- modules/database/src/ioc/db/dbChannel.c | 72 +++-------- modules/database/src/ioc/db/dbChannel.h | 8 +- modules/database/src/ioc/db/dbEvent.c | 69 ++++++----- modules/database/src/ioc/db/dbExtractArray.c | 70 +++-------- modules/database/src/ioc/db/dbExtractArray.h | 35 +++++- modules/database/src/ioc/db/db_field_log.h | 53 ++++---- modules/database/src/std/filters/arr.c | 116 +++++++----------- modules/database/src/std/filters/ts.c | 41 +++++-- modules/database/test/ioc/db/dbChArrTest.cpp | 2 +- modules/database/test/std/filters/arrTest.cpp | 6 +- modules/database/test/std/filters/dbndTest.c | 9 +- 12 files changed, 239 insertions(+), 281 deletions(-) diff --git a/modules/database/src/ioc/db/dbAccess.c b/modules/database/src/ioc/db/dbAccess.c index d7e5d0890..39e6e9a98 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 + * may modify paddr->pfield + */ + 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; } @@ -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 b6f53f797..c71d8426f 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 1ca02bbb6..063d91aa5 100644 --- a/modules/database/src/ioc/db/dbChannel.h +++ b/modules/database/src/ioc/db/dbChannel.h @@ -65,8 +65,9 @@ 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. + * If pLog->type==dbfl_type_ref and pLog->u.r.dtor is NULL, then dbScanLock() must + * be called before accessing the data, as this indicates the data is 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 +226,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 437cb6d5d..1db01f2cd 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; } @@ -737,20 +754,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 a7dcf4d0c..197e66c3a 100644 --- a/modules/database/src/ioc/db/dbExtractArray.c +++ b/modules/database/src/ioc/db/dbExtractArray.c @@ -14,11 +14,12 @@ /* * Author: Ralph Lange * - * based on dbConvert.c + * based on dbConvert.c, see copyNoConvert * written by: Bob Dalesio, Marty Kraimer */ #include +#include #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; - - if (increment == 1 && dstSize == srcSize) { - nUpperPart = nRequest < no_elements - offset ? nRequest : no_elements - offset; - memcpy(pdst, &psrc[offset * srcSize], dstSize * 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'; - } else { - for (; nRequest > 0; nRequest--, pdst += dstSize, 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; + /* assert preconditions */ + assert(nRequest >= 0); + assert(no_elements >= 0); + assert(increment > 0); + assert(0 <= offset); + assert(offset < no_elements); if (increment == 1) { - memcpy(pdst, &psrc[offset * srcSize], dstSize * nRequest); - if (isString) - for (i = 1; i <= nRequest; i++) - pdst[dstSize*i] = '\0'; + long nUpperPart = + nRequest < no_elements - offset ? nRequest : no_elements - offset; + memcpy(pdst, psrc + (offset * field_size), field_size * nUpperPart); + if (nRequest > nUpperPart) + memcpy(pdst + (field_size * nUpperPart), psrc, + field_size * (nRequest - nUpperPart)); } else { - for (; nRequest > 0; nRequest--, pdst += srcSize, offset += increment) { - memcpy(pdst, &psrc[offset*srcSize], dstSize); - if (isString) pdst[dstSize] = '\0'; + for (; nRequest > 0; nRequest--, pdst += field_size, offset += increment) { + offset %= no_elements; + 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 b44bd15fc..9755ac570 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 5a40f5fad..a043899ae 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, then this means the array data is still owned + * by a record. */ 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,37 +114,14 @@ 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; } u; } 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. - */ - #ifdef __cplusplus } #endif diff --git a/modules/database/src/std/filters/arr.c b/modules/database/src/std/filters/arr.c index cfa7118bf..5f251844a 100644 --- a/modules/database/src/std/filters/arr.c +++ b/modules/database/src/std/filters/arr.c @@ -13,16 +13,14 @@ #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#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; @@ -94,78 +94,56 @@ static long wrapArrayIndices(long *start, const long increment, long *end, 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 */ + /* TODO Treat scalars as arrays with 1 element */ 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; - } - 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 8a07c3f2f..0feadb0f5 100644 --- a/modules/database/src/std/filters/ts.c +++ b/modules/database/src/std/filters/ts.c @@ -12,21 +12,44 @@ */ #include +#include +#include -#include -#include -#include -#include +#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 90702b52e..9965852a8 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 bd83bd8ab..dfbbf463f 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 e7897e28a..0206865fb 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);