[lvc-project] [PATCH v1 1/3] net/ethtool/ioctl: return -EOPNOTSUPP if we have no phy stats

[Daniil Tatianin]

On 11/25/22 10:02 PM, Andrew Lunn wrote:
> On Fri, Nov 25, 2022 at 07:49:11PM +0300, Daniil Tatianin wrote:
>> It's not very useful to copy back an empty ethtool_stats struct and
>> return 0 if we didn't actually have any stats. This also allows for
>> further simplification of this function in the future commits.
>>
>> Signed-off-by: Daniil Tatianin <d-tatianin at yandex-team.ru>
>> ---
>>   net/ethtool/ioctl.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
>> index 57e7238a4136..071e9bf16007 100644
>> --- a/net/ethtool/ioctl.c
>> +++ b/net/ethtool/ioctl.c
>> @@ -2089,11 +2089,15 @@ static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)
>>   		n_stats = phy_ops->get_sset_count(phydev);
>>   	else
>>   		n_stats = ops->get_sset_count(dev, ETH_SS_PHY_STATS);
>> +
>>   	if (n_stats < 0)
> 
> Please don't make unneeded white space changes. It just distracts from
> the real change being made here.
Thought it looked a bit too bulky and not as readable with a new if 
statement added at the end. I don't mind removing it though.
> 
>>   		return n_stats;
>>   	if (n_stats > S32_MAX / sizeof(u64))
>>   		return -ENOMEM;
>> -	WARN_ON_ONCE(!n_stats);
>> +	if (!n_stats) {
>> +		WARN_ON_ONCE(1);
>> +		return -EOPNOTSUPP;
>> +	}
> 
> WARN_ON_ONCE() returns the result of the comparison being made. so you can do:
> 
> 	if (WARN_ON_ONCE(!n_stats))
> 		return -EOPNOTSUPP;
> 
> 	Andrew		
>
Good to know, thanks!