[Accel-config] Re: [PATCH v1] accel-config: Skip printing error if cmd status register is not set

On 2/23/2021 3:39 PM, Thomas, Ramesh wrote:
> On Tue, Feb 23, 2021 at 02:39:49PM -0700, Dave Jiang wrote:
>> On 2/23/2021 2:14 PM, ramesh.thomas(a)intel.com wrote:
>>> From: Ramesh Thomas <ramesh.thomas(a)intel.com>
>>>
>>> Library functions do their own checking and return errors without
>>> issuing commands to the hardware. Command status register retrieved
>>> from "cmd_status" sysfs attribute may not be set in such cases. Return
>>> NULL from function that retrieves the error status to indicate this.
>>>
>>> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
>>> ---
>>>    accfg/lib/libaccfg.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c
>>> index ae862f8..7d5e9f4 100644
>>> --- a/accfg/lib/libaccfg.c
>>> +++ b/accfg/lib/libaccfg.c
>>> @@ -1469,7 +1469,7 @@ ACCFG_EXPORT const char * accfg_device_get_cmd_status_str(struct accfg_device *d
>>>    const char *stat_str;
>>>
>>>    status = accfg_device_get_cmd_status(device);
>>> -if (status < 0)
>>> +if (status <= 0)
>> Would it be better to just return CMD_SUCCESS or something?
> This function returns a string error message. The calling places assume
> that if the string is not NULL then it is an error. They don't display
> the message if it is NULL. I do like the idea of it returning 0 =
> Success. I can make this function return only the integer error code and
> create another function to return the string. That way the caller can
> decide whether or not to display the error message if it is "Success".

Sounds good!


>
>>
>>>    return NULL;
>>>
>>>    if (status > ACCFG_CMD_STATUS_MAX)