[dpdk-dev] [PATCH v4 3/3] ethdev: allow close function to return an error
Ferruh Yigit
ferruh.yigit at intel.com
Tue Oct 13 11:33:23 CEST 2020
On 10/13/2020 9:55 AM, Thomas Monjalon wrote:
<...>
>>>>> dev = &rte_eth_devices[port_id];
>>>>>
>>>>> - RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_close);
>>>>> - (*dev->dev_ops->dev_close)(dev);
>>>>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_close, -ENOTSUP);
>>>>> + *lasterr = (*dev->dev_ops->dev_close)(dev);
>>>>> + if (*lasterr != 0)
>>>>> + lasterr = &binerr;
>>>>>
>>>>> rte_ethdev_trace_close(port_id);
>>>>> - rte_eth_dev_release_port(dev);
>>>>> + *lasterr = rte_eth_dev_release_port(dev);
>>>>> +
>>>>> + return firsterr;
>>>>> }
>>>>
>>>> This may be personal taste but above error handling looks like unnecessary
>>>> complex, what do you think something like below:
>>>>
>>>> close_err = (*dev->dev_ops->dev_close)(dev);
>>>> release_err = rte_eth_dev_release_port(dev);
>>>> return close_err ? close_err : release_err;
>>>
>>> This is what I did first. Then thought about this "elegant" handling.
>>> The pointer solution is just one more line and is more future proof.
>>>
>>> I'm fine with any choice. Andrew?
>>>
>>
>> Hm, really hard to make a choice. I tend to choose Thomas's
>> version. Yes, I agree that it is a bit over-complicated in
>> this particular case, but thoughts to be future-proof
>> overweight for me. Plus it adds a pattern on how to handle
>> such cases in the future.
>
> Yes it's an elegant pattern :)
>
OK :)
More information about the dev
mailing list