EPICS Controls Argonne National Laboratory

Experimental Physics and
Industrial Control System

1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  <20122013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024  Index 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  <20122013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
<== Date ==> <== Thread ==>

Subject: RE: strange EPICS glitch
From: Mark Rivers <[email protected]>
To: Eric Norum <[email protected]>
Cc: "[email protected] talk" <[email protected]>
Date: Mon, 12 Nov 2012 21:42:59 +0000
> But if there's no space in the waveform for the terminating '\0' I don't see how making changes

> to setStringParam is going to help either.



There IS space in the waveform record, that's not the problem.



The problem is that with the changes made recently in EPICS when a client requests a monitor callback with a size of 0.  In this case the array returned in the callback now has exactly NORD elements.  The problem is that asynPortDriver is currently setting the length of the array to strlen(array).  So NORD is set to strlen(array).  Clients thus receive arrays without a Nil terminator.



Your change to devAsynOctet does not fix this problem, because it does not increase NORD when it adds the Nil.  I argue that it cannot increase NORD because that will break support for binary data in devAsynOctet.



I think the correct solution is to change asynPortDriver (and perhaps other drivers) so that when they return strings (but not binary arrays) to the waveform record, either with pasynOctet->read() or asynOctet callbacks they set NORD to strlen(array)+1, and add the Nil terminator.



This does have a slight potential to break existing clients, because the meaning of NORD has now changed.



Mark





________________________________
From: Eric Norum [[email protected]]
Sent: Monday, November 12, 2012 3:25 PM
To: Mark Rivers
Cc: [email protected] talk
Subject: Re: strange EPICS glitch

But if there's no space in the waveform for the terminating '\0' I don't see how making changes to setStringParam is going to help either.

I was responding to  your earlier comment that, "This cannot be moved up to asynWfOctetRead/asynWfOctetWrite …".  I think that it can, and should, be with the change I proposed.

On Nov 12, 2012, at 1:04 PM, Mark Rivers <[email protected]<mailto:[email protected]>> wrote:

No, I don’t think that will fix it, because you have not increased NORD.  Any clients that use the new EPICS feature of subscribing to callbacks with a length of zero only receive NORD elements in the callback.  So the string they receive won’t be Nil terminated, even though the waveform record itself is now Nil terminated.

But you can’t increase NORD in devAsynOctet, because that device support must also be able to handle binary byte arrays, and increasing NORD would mess that up.

Mark


From: Eric Norum [mailto:[email protected]<http://lbl.gov>]
Sent: Monday, November 12, 2012 2:58 PM
To: Mark Rivers
Cc: [email protected]<mailto:[email protected]> talk
Subject: Re: strange EPICS glitch

Would this fix things?

--- devAsynOctet.c     2012-06-25 15:20:07.000000000 -0700
+++ devAsynOctet.c.proposed          2012-11-12 12:55:29.000000000 -0800
@@ -657,17 +657,21 @@
 static void callbackWfRead(asynUser *pasynUser)
 {
     devPvt         *pdevPvt = (devPvt *)pasynUser->userPvt;
     waveformRecord *pwf = (waveformRecord *)pdevPvt->precord;
     size_t         nBytesRead;
     asynStatus     status;

     status = readIt(pasynUser,pwf->bptr,pwf->nelm,&nBytesRead);
-    if(status==asynSuccess) pwf->nord = nBytesRead;
+    if(status==asynSuccess) {
+        pwf->nord = nBytesRead;
+        if (pwf->nelm > pwf->nord)
+            *((char *)pwf->bptr + pwf->nord) = '\0';
+    }
     finish((dbCommon *)pwf);
 }

 static long initWfWrite(waveformRecord *pwf)
 {
     asynStatus status;
     devPvt     *pdevPvt;


On Nov 12, 2012, at 12:43 PM, Mark Rivers <[email protected]<mailto:[email protected]>> wrote:


Hi Andrew,

I will change asynPortDriver so that the setStringParam() method adds a Nil and passes that length to device support on readOctet or asynOctet callbacks.  That will fix the problem with areaDetector, which is probably the main driver class using long strings for path names, etc.

This cannot be moved up to asynWfOctetRead/asynWfOctetWrite device support in asyn/devEpics/devAsynOctet.c, because the asynOctet support for the waveform record needs to be able to handle binary data.  However, we could add a new device support in there that was specifically for strings, e.g. asynWfOctetStringRead/asynWfOctetStringWrite.  These could enforce the Nils in both directions.

--
Eric Norum
[email protected]<mailto:[email protected]>

--
Eric Norum
[email protected]<mailto:[email protected]>






Replies:
Re: strange EPICS glitch Eric Norum
References:
Re: strange EPICS glitch Eric Norum
RE: strange EPICS glitch Mark Rivers
Re: strange EPICS glitch Eric Norum

Navigate by Date:
Prev: Re: strange EPICS glitch Eric Norum
Next: Re: strange EPICS glitch Eric Norum
Index: 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  <20122013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
Navigate by Thread:
Prev: Re: strange EPICS glitch Eric Norum
Next: Re: strange EPICS glitch Eric Norum
Index: 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  <20122013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
ANJ, 18 Nov 2013 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· Search · EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·