Please may I query the logic of asynPortDriver::callParamCallbacks(int addr)?

[pheest]

params is a 2D array, being a vector[maxAddr] of parameter lists.

I think the logic given below (for the 1D method overload) isn't correct, in that the code accesses the [addr, addr] address of the array. I believe it should be accssing the [0, addr], the 'normal' case being just one list (i.e. maxAddr=1).

This results in a 'vector subscript out of range' debug assertion, when called with an addr value of >maxAddr-1.

I think the associated comments e.g. "with list=addr, which is normal." and "Typically the same value as list" are also misleading.

(I usually call the 2D method overload, so I hadn't noticed this before.)

/** Calls callParamCallbacks(addr, addr) i.e. with list=addr, which is normal. */ asynStatus asynPortDriver::callParamCallbacks(int addr) { return this->callParamCallbacks(addr, addr); }

/** Calls paramList::callCallbacks(addr) for a specific parameter list.

• \param[in] list The parameter list number. Must be < maxAddr passed to asynPortDriver::asynPortDriver.
• \param[in] addr The asyn address to be used in the callback. Typically the same value as list. */ asynStatus asynPortDriver::callParamCallbacks(int list, int addr) { return this->params[list]->callCallbacks(addr); }

[MarkRivers]

I think the logic given below (for the 1D method overload) isn't correct, in that the code accesses the [addr, addr] address of the array.

I don't think that is true. On what line does it access the [addr,addr] address of the array? callParamCallbacks() always does callbacks for all parameters in the selected list, not on a specific parameter.

asynPortDriver has a separate parameter list for each asyn "addr". When it does callbacks on a particular list it is told what asyn addr address to pass in the callback. But that is actually redundant, because the implementation of asynPortDriver actually assumes and requires that list N be associated with asyn addr N. Thus it would never make sense to call callParamCallbacks(1,2), it should always be (1,1) or (2,2).

The base class implementation of writeInt32, for example, always writes the value for asyn addr N to list N.

I believe the 2 argument version of callParamCallbacks is actually redundant, and should not be used, because it is a requirement that the two values be the same. One argument select the list, the other selects the addr for the callback, but they must agree.

[MarkRivers]

@pheest do you have any comment on my reply?

[pheest]

Hi Mark,

I’m sorry for the delayed response. I’ve been overwhelmed by other priorities, in the last couple weeks.

My mistake (sorry) was to confuse the 2 parameters, passing e.g. (from testAsynPortDriver) like callParamCallbacks(P_MinValue). This results in indexing this->params with a value that exceeds the boundary of the vector, because P_MinValue is > 0. (No range check on the ‘list’ value, other than std::vector’s own.)

Params is a vector of paramList* so each element of the vector is a paramList, which in turn contains 2 vectors, flags and vals.

I would comment that parameter name use isn’t very clear or consistent. e.g: setDoubleParam has parameters (int list, int index, double value), where list is used to index the params vector (must be < maxAddr). Index is then used to index paramList::vals. The 2-parameter overload is called with list=0.

But callParamCallbacks has parameters (int list, int addr), where list is used to index the params vector (must be < maxAddr). addr is then used within methods like paramlist::int32Callback with the condition (address == addr). The 1-parameter overload calls this->callParamCallbacks(addr, addr); i.e. list=addr

I am really struggling to follow this data model.

And I’m wholly at a loss to understand the logic of “it is a requirement that the two values be the same". I understand that that’s what the comments say, though.

Best wishes,

Peter.

From: Mark Rivers [email protected] Sent: 24 September 2020 22:32 To: epics-modules/asyn [email protected] Cc: Heesterman, Peter J [email protected]; Mention [email protected] Subject: Re: [epics-modules/asyn] Please may I query the logic of asynPortDriver::callParamCallbacks(int addr)? (#124)

@pheesthttps://github.com/pheest do you have any comment on my reply?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/epics-modules/asyn/issues/124#issuecomment-698599271, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD6SZN2ZDKDDIVAEIIH5UI3SHO3ETANCNFSM4RO265SA.