[dpdk-dev] [PATCH v4 3/3] ethdev: allow close function to return an error

[Ferruh Yigit]

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 :)