Code review comment for lp:~matej-sekoranja/epics-base/pcas-variable-length-arrays

Revision history for this message
Bruce Hill (bhill) wrote :

Sorry I'm so late to this party. Murali asked me to take a look at this last
summer and I didn't like the need to change the PV::read() to avoid getting just
one element on the first read. Then I got busy with other stuff and forgot
about this merge proposal altogether.

I've been wanting to find a fix for the var array issue w/ our gateways for some
time and finally dug into it Monday and found a fix. I was going to push it
to launchpad only to rediscover this merge from Matej.

My initial fix was to clip the number of elements to the current array size
in casStrmClient.cc, but I couldn't figure out why m_count was still coming in
w/ the full array size even when I specified zero. Turns out I'd neglected to
bump up EPICS_PROTOCOL_MINOR_VERSION in caHdrLargeArray.h.

After comparing our solutions, which were nearly the same, I boiled it down
to the following fix which creates the gdd w/ the max array size when zero is
passed in, thus avoiding the problem of no bounds.

=== modified file 'src/ca/legacy/pcas/generic/casStrmClient.cc'
--- src/ca/legacy/pcas/generic/casStrmClient.cc 2015-04-01 07:29:16 +0000
+++ src/ca/legacy/pcas/generic/casStrmClient.cc 2016-07-14 08:59:50 +0000
@@ -2612,10 +2612,16 @@
 {
     const caHdrLargeArray * pHdr = this->ctx.getMsg();

+ ca_uint32_t count = pHdr->m_count;
+ if ( count == 0 ) {
+ if ( this->ctx.getChannel() ) {
+ count = this->ctx.getChannel()->getPVI().nativeCount();
+ }
+ }
+
     {
         gdd * pDD = 0;
- caStatus status = createDBRDD ( pHdr->m_dataType,
- pHdr->m_count, pDD );
+ caStatus status = createDBRDD ( pHdr->m_dataType, count, pDD );
         if ( status != S_cas_success ) {
             return status;
     }

I've submitted this as a merge proposal to Matej's branch so it can be
brought in through his branch if everyone agrees.

I've tested this w/ a gateway app and it always returns the actual array size
when m_count is set to zero.

« Back to merge proposal