diff --git a/documentation/RELEASE_NOTES.md b/documentation/RELEASE_NOTES.md index 6de124271..ffee7d83a 100644 --- a/documentation/RELEASE_NOTES.md +++ b/documentation/RELEASE_NOTES.md @@ -18,6 +18,21 @@ should also be read to understand what has changed since earlier releases. +### Fix aai's Device Support Initialization + +Krisztian Loki [reported](https://github.com/epics-base/epics-base/issues/97) +segfaults occurring when a Soft Channel aai record INP field was a DB link to +an array field of a compress record. This was caused by the aai record's +pass-0 device support initialization clashing with the semantics of the new +link support API. + +The aai record +[has been modified](https://github.com/epics-base/epics-base/pull/114) to +allow the Soft Channel device support to request a pass-1 initialization +callback. See the Device Support section of the Array Analogue Input Record +Reference pages in this release for the API changes, which are fully backwards +compatible for existing aai device support. + ### Prevent default DTYPs from changing [Kay Kasemir reported](https://bugs.launchpad.net/epics-base/+bug/1908305) that diff --git a/modules/database/src/std/dev/devAaiSoft.c b/modules/database/src/std/dev/devAaiSoft.c index 55975bae0..f2e4a41f9 100644 --- a/modules/database/src/std/dev/devAaiSoft.c +++ b/modules/database/src/std/dev/devAaiSoft.c @@ -47,19 +47,15 @@ static long init_record(dbCommon *pcommon) aaiRecord *prec = (aaiRecord *)pcommon; DBLINK *plink = &prec->inp; - /* This is pass 0, link hasn't been initialized yet */ - dbInitLink(plink, DBF_INLINK); + /* Ask record to call us in pass 1 instead */ + if (prec->pact != AAI_DEVINIT_PASS1) { + return AAI_DEVINIT_PASS1; + } if (dbLinkIsConstant(plink)) { long nRequest = prec->nelm; long status; - /* Allocate a buffer, record support hasn't done that yet */ - if (!prec->bptr) { - prec->bptr = callocMustSucceed(nRequest, dbValueSize(prec->ftvl), - "devAaiSoft: buffer calloc failed"); - } - status = dbLoadLinkArray(plink, prec->ftvl, prec->bptr, &nRequest); if (!status) { prec->nord = nRequest; diff --git a/modules/database/src/std/rec/aaiRecord.c b/modules/database/src/std/rec/aaiRecord.c index c1410067c..756b892ab 100644 --- a/modules/database/src/std/rec/aaiRecord.c +++ b/modules/database/src/std/rec/aaiRecord.c @@ -112,16 +112,18 @@ static long init_record(struct dbCommon *pcommon, int pass) prec->ftvl = DBF_UCHAR; prec->nord = (prec->nelm == 1); - /* we must call pdset->init_record in pass 0 - because it may set prec->bptr which must - not change after links are established before pass 1 - */ - + /* call pdset->init_record() in pass 0 so it can do its own + * memory allocation and set prec->bptr, which must be set by + * the end of pass 0. + */ if (pdset->common.init_record) { long status = pdset->common.init_record(pcommon); - /* init_record may set the bptr to point to the data */ - if (status) + if (status == AAI_DEVINIT_PASS1) { + /* requesting pass 1 callback, remember to do that */ + prec->pact = AAI_DEVINIT_PASS1; + } + else if (status) return status; } if (!prec->bptr) { @@ -132,6 +134,14 @@ static long init_record(struct dbCommon *pcommon, int pass) return 0; } + if (prec->pact == AAI_DEVINIT_PASS1) { + /* device support asked for an init_record() callback in pass 1 */ + long status = pdset->common.init_record(pcommon); + if (status) + return status; + prec->pact = FALSE; + } + recGblInitSimm(pcommon, &prec->sscn, &prec->oldsimm, &prec->simm, &prec->siml); /* must have read_aai function defined */ diff --git a/modules/database/src/std/rec/aaiRecord.dbd.pod b/modules/database/src/std/rec/aaiRecord.dbd.pod index 8eb8bad05..969f5800f 100644 --- a/modules/database/src/std/rec/aaiRecord.dbd.pod +++ b/modules/database/src/std/rec/aaiRecord.dbd.pod @@ -151,10 +151,15 @@ for more information on simulation mode and its fields. static long init_record(aaiRecord *prec, int pass) -If device support includes C, it is called. +If device support includes an C routine it is called, but unlike +most record types this occurs in pass 0, which allows the device support to +allocate the array buffer itself. + +Since EPICS 7.0.5 the device support may return C to request +a second call to its C routine in pass 1. Checks if device support allocated array space. If not, space for the array is -allocated using NELM and FTVL. The array address is stored in the record. +allocated using NELM and FTVL. The array address is stored in BPTR. This routine initializes SIMM with the value of SIML if SIML type is CONSTANT link or creates a channel access link if SIML type is PV_LINK. VAL is likewise @@ -294,10 +299,11 @@ Scan forward link if necessary, set PACT FALSE, and return. %/* Declare Device Support Entry Table */ %struct aaiRecord; %typedef struct aaidset { - % dset common; /*init_record returns: (-1,0)=>(failure,success)*/ + % dset common; /*init_record returns: (-1,0,AAI_DEVINIT_PASS1)=>(failure,success,callback)*/ % long (*read_aai)(struct aaiRecord *prec); /*returns: (-1,0)=>(failure,success)*/ %} aaidset; %#define HAS_aaidset + %#define AAI_DEVINIT_PASS1 2 % field(VAL,DBF_NOACCESS) { prompt("Value") @@ -469,8 +475,19 @@ with C set to 1. long init_record(dbCommon *precord) -This routine is optional. If provided, it is called by the record support -C routine. +This routine is optional. +If provided, it is called by the record support's C routine in +pass 0. +The device support may allocate memory for the VAL field's array (enough space +for NELM elements of type FTVA) from its own memory pool if desired, and store +the pointer to this buffer in the BPTR field. +The record will use C for this memory allocation if BPTR has not been +set by this routine. +The routine must return 0 for success, -1 or a error status on failure. + +Since EPICS 7.0.5 if this routine returns C in pass 0, it +will be called again in pass 1 with the PACT field set to C. +In pass 0 the PACT field is set to zero (FALSE). =head4 get_ioint_info @@ -485,7 +502,8 @@ provided for any device type that can use the ioEvent scanner. long read_aai(dbCommon *precord) -This routine must provide a new input value. It returns the following values: +This routine should provide a new input value. +It returns the following values: =over @@ -501,16 +519,15 @@ Other: Error. =head3 Device Support For Soft Records -The C<<< Soft Channel >>> device support module is provided to read values from -other records and store them in arrays. If INP is a constant link, then read_aai -does nothing. In this case, the record can be used to hold arrays written via -dbPuts. If INP is a database or channel access link, the new array value is read -from the link. NORD is set. +The C<<< Soft Channel >>> device support is provided to read values from other +records via the INP link, or to hold array values that are written into it. -This module places a value directly in VAL and NORD is set to the number of items -in the array. +If INP is a constant link the array value gets loaded from the link constant by +the C routine, which also sets NORD. +The C routine does nothing in this case. -If the INP link type is constant, then NORD is set to zero. +If INP is a database or channel access link, the C routine gets a +new array value from the link and sets NORD. =cut } diff --git a/modules/database/src/std/rec/compressRecord.c b/modules/database/src/std/rec/compressRecord.c index 98e17985f..d7da7f9b6 100644 --- a/modules/database/src/std/rec/compressRecord.c +++ b/modules/database/src/std/rec/compressRecord.c @@ -106,7 +106,7 @@ static void monitor(compressRecord *prec) db_post_events(prec, &prec->nuse, monitor_mask); prec->ouse = prec->nuse; } - db_post_events(prec, prec->bptr, monitor_mask); + db_post_events(prec, (void*)&prec->val, monitor_mask); } static void put_value(compressRecord *prec, double *psource, int n) @@ -404,7 +404,6 @@ static long cvt_dbaddr(DBADDR *paddr) { compressRecord *prec = (compressRecord *) paddr->precord; - paddr->pfield = prec->bptr; paddr->no_elements = prec->nsam; paddr->field_type = DBF_DOUBLE; paddr->field_size = sizeof(double); @@ -426,6 +425,8 @@ static long get_array_info(DBADDR *paddr, long *no_elements, long *offset) epicsUInt32 off = prec->off; epicsUInt32 nuse = prec->nuse; + paddr->pfield = prec->bptr; + if (prec->balg == bufferingALG_FIFO) { epicsUInt32 nsam = prec->nsam; diff --git a/modules/database/src/std/rec/histogramRecord.c b/modules/database/src/std/rec/histogramRecord.c index 0e9724cb6..44b278f39 100644 --- a/modules/database/src/std/rec/histogramRecord.c +++ b/modules/database/src/std/rec/histogramRecord.c @@ -111,7 +111,7 @@ static void wdogCallback(epicsCallback *arg) if (prec->mcnt > 0){ dbScanLock((struct dbCommon *)prec); recGblGetTimeStamp(prec); - db_post_events(prec, prec->bptr, DBE_VALUE | DBE_LOG); + db_post_events(prec, (void*)&prec->val, DBE_VALUE | DBE_LOG); prec->mcnt = 0; dbScanUnlock((struct dbCommon *)prec); } @@ -291,7 +291,7 @@ static void monitor(histogramRecord *prec) } /* send out monitors connected to the value field */ if (monitor_mask) - db_post_events(prec, prec->bptr, monitor_mask); + db_post_events(prec, (void*)&prec->val, monitor_mask); return; } @@ -300,7 +300,6 @@ static long cvt_dbaddr(DBADDR *paddr) { histogramRecord *prec = (histogramRecord *) paddr->precord; - paddr->pfield = prec->bptr; paddr->no_elements = prec->nelm; paddr->field_type = DBF_ULONG; paddr->field_size = sizeof(epicsUInt32); @@ -312,6 +311,7 @@ static long get_array_info(DBADDR *paddr, long *no_elements, long *offset) { histogramRecord *prec = (histogramRecord *) paddr->precord; + paddr->pfield = prec->bptr; *no_elements = prec->nelm; *offset = 0; return 0; diff --git a/modules/database/src/std/rec/subArrayRecord.c b/modules/database/src/std/rec/subArrayRecord.c index 82e543ca7..b73dee16b 100644 --- a/modules/database/src/std/rec/subArrayRecord.c +++ b/modules/database/src/std/rec/subArrayRecord.c @@ -161,7 +161,6 @@ static long cvt_dbaddr(DBADDR *paddr) { subArrayRecord *prec = (subArrayRecord *) paddr->precord; - paddr->pfield = prec->bptr; paddr->no_elements = prec->malm; paddr->field_type = prec->ftvl; paddr->field_size = dbValueSize(prec->ftvl); @@ -174,6 +173,7 @@ static long get_array_info(DBADDR *paddr, long *no_elements, long *offset) { subArrayRecord *prec = (subArrayRecord *) paddr->precord; + paddr->pfield = prec->bptr; if (prec->udf) *no_elements = 0; else @@ -293,7 +293,7 @@ static void monitor(subArrayRecord *prec) monitor_mask = recGblResetAlarms(prec); monitor_mask |= (DBE_LOG|DBE_VALUE); - db_post_events(prec, prec->bptr, monitor_mask); + db_post_events(prec, (void*)&prec->val, monitor_mask); return; }