Empty arrays have undefined behavior

[Dirk Zimoch]

The behavior of arrays with 0 elements is inconsistent and seemingly undefined. I made the fllowing observations:

1. Reading empty arrays with caget shows NELM values, all 0 except for the first which sometimes contains strange not-quite-random numbers (see bug report 1242919). I have observed values 0, 2.122e-314, 2.50321e-308, 22, 18, 2.02038e-39, 1441792, 1179648, 3.05948e-308, also depending on the -d option (DBR type) of caget.

2. Reading empty arrays with camonitor shows no values.

3. A scalar input record reading INP or scalar output record reading DOL (with OMSL closed_loop) from an empty array (aai, aao or waveform) does not change its value and does not show an alarm (tested with ai, bi, mbbi, mbbiDirect, longin, stringin, ao, bo, mbbo, mboDirect, longout, stringout). This is true for DB links as well as for CA, CP and CPP links.

4. An array record (aai, waveform) reading from an empty array does not change its current value. It does not change NORD, it does not clear the arrays elements, it does not show an alarm.

5. Writing 0 elements to an array with caput -a fails whith "Invalid element count requested".

My opinion on this behavior is as follows:

1. Reading arrays with caget should return NORD elements, not NELM elements. But if we put this side, the additional elements should all be set to 0. For empty arrays, this includes the the first one (index 0).

2. This is correct behavior.

3. This is consistent but questionable. Converting an empy array to a scalar should fail as there is no value to convert. But that might break existing dbs. In that case this behavior should be documented. (Or is it?)

4. This behavior is buggy. The receiving record should update its NORD to 0.

5. It should be possible to write an empty array.

[Ralph Lange (ralph-lange)]

Problem 1. is partially handled in https://bugs.launchpad.net/epics-base/+bug/1242919 - or rather not, considering it's been there for 8 years.

[mdavidsaver (mdavidsaver)]

The undefined part of #1 could be address by fixing dbr_size_n(). Currently 'dbr_size_n(X,0) == dbr_size_n(X,1)'. Further work would involve a protocol change (why not use PVA).

I see #4 as a bug in the various Soft Channel device supports.

I've never noticed #5, though this also seems like an issue. w/ CA, CMD_PUT w/ count zero is not ambiguous.

[Andrew Johnson (anj)]

The array length part of #1 is a fundamental limitation of using caget (which calls ca_array_get()) instead of 'caget -c' => ca_array_get_callback() since only the latter API provides a way to return the actual element count to the application code. Did I read in email that someone has a fix for #1242919? Applying that would ensure the first element does get returned as zero to a ca_get() caller, but fixing the length part would require changing the ca_array_get() API, the result of which would probably look just like ca_array_get_callback(). IIRC 'caget -c' doesn't show the random first element anyway, so I see #1 as just a duplicate of https://bugs.launchpad.net/epics-base/+bug/1242919

#2 is correct behavior. Using the -F option to camonitor (and 'caget -c') might make that more obvious (IIRC there should be no field separators at all for an empty array).

#3 The scalar device supports call dbGetLink() with nRequest=NULL (which is short-hand for 1 element). For a DB link pointing to an array with zero elements that should end up calling dbGet() and that will use of the dbFastGetConvertRoutine[][] routines to copy and convert the first element from the source array (even when that source nominally has zero elements). Thus I'm not sure that your description of this bug is quite right, you need to initialize the source array to have a non-zero first element, then set its array length to 0 without overwriting the data buffer. Might be easiest to do that using an aSub record.

#4 devAaiSoft.c calls dbGetLink() to fetch the data, but only updates NORD if nRequest>0 on return. devWfSoft.c does the same thing in a slightly different way. Both also have nelm>0 checks in their init_record() routines so constant input array initialization to an empty array won't ever set NORD to zero (which it defaults to anyway) or clear UDF (which is the detectable bug in this code). We should also test the subarray record, its soft-device support also has similar code, and the aSub record also only updates the array length of its input fields if nRequest>0.

#5 We don't know yet whether this is just an issue with the caput program, or whether the underlying CA client library is unable to do zero-length puts. Can you try using an aao or aSub record with a CA output link to write a zero-length to a non-empty array record to see whether the destination gets its length set to 0 please.

[Dirk Zimoch (dirk.zimoch)]

I have started a branch to work on this:
https://code.launchpad.net/~dirk.zimoch/epics-base/+git/epics-base/+ref/fix_zero_size_arrays

#1 seems eazy enough to fix. I fixed dbr_size_n as Michael suggested and caget has no problem any more with the first element. All tests still run successfully.

#4 required to implement proper constant input link handling for aai and waveform but then worked. subArray and aSub were straight forward. All tests still run successfully.

I noticed that dbpf cannot clear an array eiter (nor can it set more than 1 element)

[mdavidsaver (mdavidsaver)]

What I see so far on your fix_zero_size_arrays branch looks straightforward and reasonable. In short, something which could be merged by itself soon.

[Dirk Zimoch (dirk.zimoch)]

I just found that INP with arrays of 1 element seems buggy:
record(aai,"aai") {
    field(NELM, 10)
    field(FTVL, "DOUBLE")
    field(INP, [42])
}
record(ai, "ai") {
    field(INP, [42])
}

epics> dbgf src
epics> dbgf src.NORD
DBF_ULONG: 0 = 0x0
epics> dbgf ai
DBF_DOUBLE: 0

I'll have a look at this, too.

[Dirk Zimoch (dirk.zimoch)]

dbpf with array support added.

[Dirk Zimoch (dirk.zimoch)]

For field(INP, [42]) see merge request 385269.

[Dirk Zimoch (dirk.zimoch)]

#5 Was a problem in the server methods nciu::write(). Fixed.

[Dirk Zimoch (dirk.zimoch)]

#3 Inddeed, sclar records reading from empty arrays do not get the value 0 but instead keep their old value (without setting any type of error). This is clearly buggy behavior.

[Dirk Zimoch (dirk.zimoch)]

#3 Behaves the same for DB links and CA links. I will change it to cause INVALID/LINK alarm.

[Andrew Johnson (anj)]

To implement a constant link with an array of zero or one elements, use the JSON const link type like this:

field(INP, {const:[]})
field(INP, {const:[1]})

[Dirk Zimoch (dirk.zimoch)]

I have now fixed:
# caget from empty arrays no longer returns nonense in first element but 0.
Warning: This will break calling dbr_size_n(TYPE,COUNT) with COUNT=0 when meaning 1. Base does not do this but 3rd pary software may.
# Array records reading empty arrays now report 0 bytes read (NORD or similar)
Fixed for aai, wavevorm, subArray, aSub. There may be more (compress?) but I have not checked yet.
# dbpf can now put arrays, including empty arrays
# caput can now put empty arrays
# scalar records reading from empty arrays via db link now get INVALID/LINK alarm.
Does not work yet for CA links.

I will be on vacation next week (from now on).
I am not creating a mrege request yet as this is not complete.

I took the change for 1-element-constant input links out of this branch https://code.launchpad.net/~dirk.zimoch/epics-base/+git/epics-base/+ref/fix_zero_size_arrays

[Ben Franksen (bfrk)]

> #3 Inddeed, sclar records reading from empty arrays do not get the value 0 but instead keep their old value (without setting any type of error). This is clearly buggy behavior.

The record should be set to to UDF/INVALID or perhaps LINK/INVALID and the value should not be updated. Updating the value to 0 is clearly wrong.

[Dirk Zimoch (dirk.zimoch)]

The change is to LINK/INVALID alarm. The value is not updated. That was the case before, it just was 0 initially.

[mdavidsaver (mdavidsaver)]

Dirk, Do you plan to create a merge request soon?

In f8035d8d5eb19501b729efd3369a002be94aa7bc

You add two 'epicsOldString *array' locals in different scopes of dbpf().
The dbCalloc() is made to the inner scope, and the free() in the outer scope.
I think this will leak.

[Dirk Zimoch (dirk.zimoch)]

I was on vacation. Now I can continue to work on this.
The inner array variable was a mistake. Left over from moving the varible to the outer scope so that I can free it later. Fixed.
I have now created merge request 386175.