Experimental Physics and
Industrial Control System

Andrew Johnson <[email protected]> · Fri, 12 Nov 2010 15:22:21 -0500

Hi Michael,

There are 3 record support routines that deal with information about array 
fields for the dbAccess routines:

* cvt_dbaddr(*paddr) is called during channel lookup, and is responsible for 
describing the array; the paddr->no_elements field must be set to the maximum 
number of elements that can ever be stored in the array's buffer.  CA uses 
that as the native size of the channel; I have no idea what might happen if 
you later tell it that the array is bigger than this number.
* get_array_info(*paddr, *no_elements, *offset) is called at the start of a 
get operation to find out the current size of the array, and if it's a 
circular buffer what the current offset is to the origin.
* put_array_info(*paddr, nNew) is called *after* dbAccess has written new data 
to the array, to update the array size.

During a put operation dbAccess relies on the information it got from the 
connection-time call to cvt_dbaddr() to tell it how many elements it may copy 
into the array.  The dbAccess routine dbPut() does not communicate with the 
record before directly overwriting the array buffer, using the information in 
its dbAddr structure.  It only calls put_array_info() to set the data size 
after writing the data.  If the dbAddr says the array size is zero, dbPut() 
will only write one element.

On Friday 12 November 2010 04:25:32 Michael Abbott wrote:
>
> Unfortunately patch [email protected]
> changes the length reported by subArray to return MALM.  Looking at this
> patch I suspect this was an oversight, but one that hasn't been noticed
> for three years!

When you say "the length reported by subArray" you are talking about the value 
returned by cvt_dbaddr().  As described above, this routine *must* return MALM 
as that's size of the array buffer and someone could later set INDX=0 and 
NELM=MALM.

> The patch below, I believe, fixes this.  cvt_dbaddr() now reports NORD as
> the true size of the available data, which makes this record type a bit
> more useful.

No, the size of the available data is returned by get_array_info(), and the 
current code for that routine correctly returns NORD, or zero if the record is 
marked as undefined.

> I discovered this change when a 4000 point record I was using to index
> into a 30000 point waveform exploded in size (and refused to update,
> because EPICS_CA_MAX_ARRAY_BYTES was not set) after upgrading from EPICS
> 3.14.8.2 to 3.14.11.

I don't quite follow what you're saying happened there, please elaborate if 
you feel I've missed something in my discussions below.

> diff --git a/src/dev/softDev/devSASoft.c b/src/dev/softDev/devSASoft.c
> index 5600ac1..32b5ad7 100644
> --- a/src/dev/softDev/devSASoft.c
> +++ b/src/dev/softDev/devSASoft.c
> @@ -81,6 +81,8 @@ static long read_sa(subArrayRecord *prec)
>          memmove(prec->bptr, (char *)prec->bptr + prec->indx * esize,
>                  ecount * esize);
>      }
> +    else
> +        prec->nord = 0;

The above change is fixing a bug, although I'm going to write it slightly 
differently so as to set NORD only once in the code I commit.  Without this 
change if the sub-array's INDX points beyond the end of the current target 
array, NORD will be set to the unsigned representation of a negative number, 
which is obviously a (big) mistake.  [Sorry, couldn't resist that one!]

> diff --git a/src/rec/subArrayRecord.c b/src/rec/subArrayRecord.c
> index 081304e..bc00d69 100644
> --- a/src/rec/subArrayRecord.c
> +++ b/src/rec/subArrayRecord.c
> @@ -168,7 +168,7 @@ static long cvt_dbaddr(DBADDR *paddr)
>      subArrayRecord *prec = (subArrayRecord *) paddr->precord;
>
>      paddr->pfield = prec->bptr;
> -    paddr->no_elements = prec->malm;
> +    paddr->no_elements = prec->nord;
>      paddr->field_type = prec->ftvl;
>      paddr->field_size = dbValueSize(prec->ftvl);
>      paddr->dbr_field_type = prec->ftvl;

This change is wrong, it's telling lies about the field, and it enables the "I 
don't know what would happen if" scenario that I described above.  It also 
prevents a DB link or a transient CA client from ever being able to write more 
than a single element to the VAL field of an empty subArray record, because 
when the link is created NORD will be zero, which the dbPut() routine takes to 
mean that it can write one element.

If a subArray's INP field is not set (prec->inp.type == CONSTANT), the record 
could be used to extract a sub-array from any data that gets put into its VAL 
field â processing the record triggers the extraction code in the device 
support's read_sa() routine.  I believe it would be useful to be able to write 
arrays into the VAL field, although unfortunately that only currently works if 
you point its INP field at a non-existent record.  That is easily fixed 
though, so in R3.14.12-pre2-DEV I can now do this:

epics> dbpr anjHost:saExample 1
ASG:                BKPT: 00            BUSY: 0             DESC:
DISA: 0             DISP: 0             DISS: NO_ALARM      DISV: 1
DTYP: Soft Channel  EGU:                EVNT: 0             FLNK:CONSTANT 0
FTVL: STRING        HOPR: 0             INDX: 2             INP:CONSTANT
LOPR: 0             MALM: 10            NAME: anjHost:saExample
NELM: 8             NORD: 0             PACT: 0             PHAS: 0
PINI: NO            PREC: 0             PRIO: LOW           PUTF: 0
RPRO: 0             SCAN: Passive       SDIS:CONSTANT       SEVR: INVALID
STAT: UDF           TPRO: 0             TSE: 0              TSEL:CONSTANT
UDF: 1              VAL: (nil)

tux% caput -a anjHost:saExample 10 1 2 3 4 5 6 7 8 9 10
Old : anjHost:saExample 10
New : anjHost:saExample 10 3 4 5 6 7 8 9 10
tux% caput -t anjHost:saExample.PROC 1
1
tux% caget anjHost:saExample
anjHost:saExample 10 5 6 7 8 9 10
tux% caput -t anjHost:saExample.PROC 1
1
tux% caget anjHost:saExample
anjHost:saExample 10 7 8 9 10
tux% caput -t anjHost:saExample.PROC 1
1
tux% caget anjHost:saExample
anjHost:saExample 10 9 10
tux% caput -t anjHost:saExample.PROC 1
1
tux% caget anjHost:saExample
anjHost:saExample 10

epics> dbpr anjHost:saExample
ASG:                BUSY: 0             DESC:               DISA: 0
DISP: 0             DISV: 1             INDX: 2
NAME: anjHost:saExample                 NELM: 8             NORD: 0
SEVR: INVALID       STAT: UDF           TPRO: 0             VAL: (nil)

Interestingly you could use this to feed a series of values into some other 
record in order; just set INDX=1 and NELM=MALM and have your other record read 
the first element of the VAL field each time.  This has lots of related 
possibilities.

> @@ -180,10 +180,7 @@ static long get_array_info(DBADDR *paddr, long
> *no_elements, long *offset) {
>      subArrayRecord *prec = (subArrayRecord *) paddr->precord;
>
> -    if (prec->udf)
> -       *no_elements = 0;
> -    else
> -       *no_elements = prec->nord;
> +    *no_elements = prec->nord;
>      *offset = 0;
>
>      return 0;

Why would you want an undefined array to be able to report that it contains 
data?  That's what this change would do.

> @@ -194,9 +191,8 @@ static long put_array_info(DBADDR *paddr, long nNew)
>      subArrayRecord *prec = (subArrayRecord *) paddr->precord;
>
>      if (nNew > prec->malm)
> -       prec->nord = prec->malm;
> -    else
> -       prec->nord = nNew;
> +        nNew = prec->malm;
> +    prec->nord = nNew;
>
>      return 0;
>  }

This change doesn't alter the functionality, but I will commit it anyway.

Thanks for the report and patch,

- Andrew
-- 
If a man is offered a fact which goes against his instincts, he will
scrutinize it closely, and unless the evidence is overwhelming, he will
refuse to believe it.  If, on the other hand, he is offered something
which affords a reason for acting in accordance to his instincts, he
will accept it even on the slightest evidence.  -- Bertrand Russell

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System