RE: Long string support in CA clients and device support

[Mark Rivers <rivers@cars.uchicago.edu>]

Folks,

I've just done some investigation to see how asynPortDriver currently handles writing and reading strings in the parameter library. 

The lowest level class is paramVal.cpp which implements setString and getString as follows:

void paramVal::setString(const char *value)
{
    if (type != asynParamOctet)
        throw ParamValWrongType("paramVal::setString can only handle asynParamOctet");
    if (!isDefined() || (strcmp(data.sval, value)))
    {
        setDefined(true);
        if (data.sval != NULL)
            free(data.sval);
        data.sval = epicsStrDup(value);
        setValueChanged();
    }
}

char* paramVal::getString()
{
    if (type != asynParamOctet)
        throw ParamValWrongType("paramVal::getString can only handle asynParamOctet");
    if (!isDefined())
        throw ParamValNotDefined("paramVal::geString value not defined");
    return data.sval;
}

So setString just calls epicsStrDup (which adds a Nil terminator) and getString just returns a pointer to the string.  No problems here.

The next class up is paramList.cpp, which implements setString and getString as follows:

asynStatus paramList::setString(int index, const char *value)
{
    if (index < 0 || index >= this->nVals) return asynParamBadIndex;
    try {
        getParameter(index)->setString(value);
        registerParameterChange(getParameter(index), index);
    }
    catch (ParamValWrongType&){
        return asynParamWrongType;
    }
    catch (ParamListInvalidIndex&) {
        return asynParamBadIndex;
    }
    return asynSuccess;
}

asynStatus paramList::getString(int index, int maxChars, char *value)
{
    asynStatus status=asynSuccess;
    
    try {
        if (maxChars > 0) {
            paramVal *pVal = getParameter(index);
            status = pVal->getStatus();
            strncpy(value, pVal->getString(), maxChars-1);
            value[maxChars-1] = '\0';
        }
    }
    catch (ParamValWrongType&){
        return asynParamWrongType;
    }
    catch (ParamValNotDefined&){
        return asynParamUndefined;
    }
    catch (ParamListInvalidIndex&) {
        return asynParamBadIndex;
    }
    return status;
}

setString just calls paramVal->setString.  getString calls strncpy, which will return a Nil terminated string if there is room.  And then it sets the last element of the array to Nil, so the returned string is guaranteed to be Nil terminated, even if the buffer was not large enough.

The next class up is asynPortDriver.  It implements setStringParam and getStringParam as follows:

asynStatus asynPortDriver::setStringParam(int list, int index, const char *value)
{
    asynStatus status;
    static const char *functionName = "setStringParam";

    status = this->params[list]->setString(index, value);
    if (status) reportSetParamErrors(status, index, list, functionName);
    return(status);
}

asynStatus asynPortDriver::getStringParam(int list, int index, int maxChars, char *value)
{
    asynStatus status;
    static const char *functionName = "getStringParam";

    status = this->params[list]->getString(index, maxChars, value);
    if (status) reportGetParamErrors(status, index, list, functionName);
    return(status);
}

So it just calls paramList::setString and paramList::getString, which were described above.

asynPortDriver also implements writeOctet and readOctet, which are the methods called by device support (via a thin C wrapper function)

asynStatus asynPortDriver::writeOctet(asynUser *pasynUser, const char *value, 
                                    size_t nChars, size_t *nActual)
{
    int addr=0;
    int function = pasynUser->reason;
    asynStatus status = asynSuccess;
    const char *functionName = "writeOctet";

    status = getAddress(pasynUser, &addr); if (status != asynSuccess) return(status);

    /* Set the parameter in the parameter library. */
    status = (asynStatus)setStringParam(addr, function, (char *)value);

     /* Do callbacks so higher layers see any changes */
    status = (asynStatus)callParamCallbacks(addr, addr);

    if (status) 
        epicsSnprintf(pasynUser->errorMessage, pasynUser->errorMessageSize, 
                  "%s:%s: status=%d, function=%d, value=%s", 
                  driverName, functionName, status, function, value);
    else        
        asynPrint(pasynUser, ASYN_TRACEIO_DRIVER, 
              "%s:%s: function=%d, value=%s\n", 
              driverName, functionName, function, value);
    *nActual = nChars;
    return status;
}

asynStatus asynPortDriver::readOctet(asynUser *pasynUser,
                            char *value, size_t maxChars, size_t *nActual,
                            int *eomReason)
{
    int function = pasynUser->reason;
    int addr=0;
    asynStatus status = asynSuccess;
    const char *functionName = "readOctet";
   
    status = getAddress(pasynUser, &addr); if (status != asynSuccess) return(status);
    /* We just read the current value of the parameter from the parameter library.
     * Those values are updated whenever anything could cause them to change */
    status = (asynStatus)getStringParam(addr, function, maxChars, value);
    if (status) 
        epicsSnprintf(pasynUser->errorMessage, pasynUser->errorMessageSize, 
                  "%s:%s: status=%d, function=%d, value=%s", 
                  driverName, functionName, status, function, value);
    else        
        asynPrint(pasynUser, ASYN_TRACEIO_DRIVER, 
              "%s:%s: function=%d, value=%s\n", 
              driverName, functionName, function, value);
    if (eomReason) *eomReason = ASYN_EOM_END;
    *nActual = strlen(value);
    return(status);
}

So writeOctet calls asynPortDriver::setStringParam.  It returns nActual=nChars, i.e. the string length that was passed in.  Note that writeOctet() is often overridden in derived classes if an action needs to be taken immediately when the string it written.

readOctet calls asynPortDriver::getStringParam.  It returns nActual=strlen(value).  readOctet is rarely overridden in derived classes, typically reading the last value from the parameter library is all that is required.

The final routine it paramList::octetCallback.  This is the function that does callbacks to device support for strings with records that have SCAN=I/O Intr.

asynStatus paramList::octetCallback(int command, int addr)
{
    ELLLIST *pclientList;
    interruptNode *pnode;
    asynStandardInterfaces *pInterfaces = this->pasynInterfaces;
    int address;
    char *value;
    asynStatus status;

    /* Pass octet interrupts */
    value = getParameter(command)->getString();
    getStatus(command, &status);
    if (!pInterfaces->octetInterruptPvt) return(asynParamNotFound);
    pasynManager->interruptStart(pInterfaces->octetInterruptPvt, &pclientList);
    pnode = (interruptNode *)ellFirst(pclientList);
    while (pnode) {
        asynOctetInterrupt *pInterrupt = (asynOctetInterrupt *) pnode->drvPvt;
        pasynManager->getAddr(pInterrupt->pasynUser, &address);
        /* Set the status for the callback */
        pInterrupt->pasynUser->auxStatus = status;
        /* If this is not a multi-device then address is -1, change to 0 */
        if (address == -1) address = 0;
        if ((command == pInterrupt->pasynUser->reason) &&
            (address == addr)) {
            pInterrupt->callback(pInterrupt->userPvt,
                                 pInterrupt->pasynUser,
                                 value, strlen(value), ASYN_EOM_END);
        }
        pnode = (interruptNode *)ellNext(&pnode->node);
    }
    pasynManager->interruptEnd(pInterfaces->octetInterruptPvt);
    return(asynSuccess);
}

It also passes the length as strlen(value).

Conclusions:

- The strings in the parameter library are guaranteed to be Nil terminated because they are only set with epicsStrDup.

- Strings passed back in readOctet and octetCallback are guaranteed to be Nil terminated.

- The lengths returned in readOctet and octetCallback are strlen(value), i.e. without the Nil.  

- Should these be changed to strlen(value)+1?  That will cause NORD to increase to be strlen(value)+1.

I'm willing to do this, but I tend to agree with Eric that it seems more logical for NORD to be the actual string length.

Mark


-----Original Message-----
From: Eric Norum [mailto:eric@norum.ca] 
Sent: Tuesday, November 13, 2012 1:20 PM
To: Andrew Johnson
Cc: Mark Rivers
Subject: Re: Long string support in CA clients and device support

The more that I think about this the more I'm unsure that increasing NORD to include a trailing '\0' is a good idea.  NORD, as its name implies, is the number of characters read.  
Existing message-based device support  of which I'm aware (asyn IP, asyn serial to mention a couple) return just the number of characters actually read -- no extra '\0' added.   So clients dealing with long string waveforms should already know that they have to deal with non-terminated strings.   Having NORD incremented in some places and not in others just seems wrong to me.

-- 
Eric Norum
eric@norum.ca