Experimental Physics and Industrial Control System
As I was fixing the string termination problem that Freddie found, I discovered that there was another problem with interruptCallbackSi and interruptCallbackWaveform in devAsynOctet.c. Neither of those functions was locking the record with dbScanLock while the record fields were being modified. All of the other asyn device support does correctly lock records during the interrupt callbacks. The problem was only in devAsynOctet.c.
I have fixed both problems and committed to SVN. The fixes will be in the R4-25 release of asyn. Meanwhile you can get them here:
https://svn.aps.anl.gov/epics/asyn/trunk
Mark
-----Original Message-----
From: Mark Rivers
Sent: Friday, November 07, 2014 10:26 AM
To: '[email protected]'; [email protected]
Subject: RE: Asyn and stringin records
Hi Freddie,
This is the current code that handles the interrupt callbacks for the stringin record. You are definitely correct that it does not guaranteed that the string is null terminated.
This is the code for periodic reads. It does guarantee that the string is null terminated.
static void callbackSiRead(asynUser *pasynUser)
{
devPvt *pdevPvt = (devPvt *)pasynUser->userPvt;
stringinRecord *psi = (stringinRecord *)pdevPvt->precord;
size_t nBytesRead;
asynStatus status;
size_t len = sizeof(psi->val);
status = readIt(pasynUser,psi->val,len,&nBytesRead);
psi->time = pasynUser->timestamp;
if(status==asynSuccess) {
psi->udf = 0;
if(nBytesRead==len) nBytesRead--;
psi->val[nBytesRead] = 0;
}
finish((dbCommon *)psi);
}
This is the code for I/O Intr scanned records. It does NOT guarantee that the string is null terminated.
static void interruptCallbackSi(void *drvPvt, asynUser *pasynUser,
char *data,size_t numchars, int eomReason)
{
devPvt *pdevPvt = (devPvt *)drvPvt;
stringinRecord *psi = (stringinRecord *)pdevPvt->precord;
size_t num;
pdevPvt->gotValue = 1;
num = (numchars>=MAX_STRING_SIZE ? MAX_STRING_SIZE : numchars);
strncpy(psi->val,data,num);
psi->udf = 0;
if(num<MAX_STRING_SIZE) psi->val[num] = 0;
/* Set the status from pasynUser->auxStatus so I/O Intr scanned records can set alarms */
if (pdevPvt->status == asynSuccess) pdevPvt->status = pasynUser->auxStatus;
psi->time = pasynUser->timestamp;
scanIoRequest(pdevPvt->ioScanPvt);
}
I propose to change this to:
static void interruptCallbackSi(void *drvPvt, asynUser *pasynUser,
char *data,size_t numchars, int eomReason)
{
devPvt *pdevPvt = (devPvt *)drvPvt;
stringinRecord *psi = (stringinRecord *)pdevPvt->precord;
size_t maxChars = sizeof(psi->val)-1;
pdevPvt->gotValue = 1;
if (numchars > maxChars) numchars = maxChars;
strncpy(psi->val,data,numchars);
psi->val[numchars] = 0;
psi->udf = 0;
/* Set the status from pasynUser->auxStatus so I/O Intr scanned records can set alarms */
if (pdevPvt->status == asynSuccess) pdevPvt->status = pasynUser->auxStatus;
psi->time = pasynUser->timestamp;
scanIoRequest(pdevPvt->ioScanPvt);
}
Mark
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of [email protected]
Sent: Friday, November 07, 2014 9:58 AM
To: [email protected]
Subject: Asyn and stringin records
Hi,
I have come across a difference in behaviour with asyn and the EPICS stringin record (asyn 4-22 / base 3.14.12.2 / win64) that occurs when the value received by the driver happens to be larger than the stringin maximum size. A record was changed from "periodic scan" to "I/O Intr" resulting in a crash due to a strcpy() (in monitor() from stringinRecord.c of base) being unable to find a NULL terminator and thus overwriting memory. Looking at devAsynOctet.c it seems that stringin records that are periodically scanned are always guaranteed to have a NULL terminated value, whereas ones processed via interruptCallbackSi() are NULL terminated only if the data received is less than 40 characters long (I noticed that in base 3.14.12.4 the stringinRecord.c monitor() routine now uses strncpy() and so does not require NULL termination). For my case I've just added NULL termination for "I/O Intr" too, but from looking at the stringin record reference manual page I wasn't sure if the ori!
ginal I/O Instr stringin behaviour was actually more correct?
Regards,
Freddie
--
Scanned by iCritical.
- Replies:
- RE: Asyn and stringin records freddie.akeroyd
- References:
- Asyn and stringin records freddie.akeroyd
- RE: Asyn and stringin records Mark Rivers
- Navigate by Date:
- Prev:
Re: Preemptive CA client that never calls ca_flush_io, ca_pend_io, ca_pend_event, nor ca_poll Andrew Johnson
- Next:
RE: Asyn and stringin records freddie.akeroyd
- Index:
1994
1995
1996
1997
1998
1999
2000
2001
2002
2003
2004
2005
2006
2007
2008
2009
2010
2011
2012
2013
<2014>
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
- Navigate by Thread:
- Prev:
RE: Asyn and stringin records Mark Rivers
- Next:
RE: Asyn and stringin records freddie.akeroyd
- Index:
1994
1995
1996
1997
1998
1999
2000
2001
2002
2003
2004
2005
2006
2007
2008
2009
2010
2011
2012
2013
<2014>
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024