Re: [PATCH RESEND] ipv6: return with appropriate error code when sending RH0 using setsockopt()

[Shan Wei <shanwei@cn.fujitsu.com>]

David Miller 写道:
> From: Shan Wei <shanwei@cn.fujitsu.com>
> Date: Thu, 04 Sep 2008 11:44:10 +0800
> 
>> Brian Haley 写道:
>>> Shan Wei wrote:
>>>> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
>>>> index 4e5eac3..7a58597 100644
>>>> --- a/net/ipv6/ipv6_sockglue.c
>>>> +++ b/net/ipv6/ipv6_sockglue.c
>>>> @@ -353,9 +353,10 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
>>>>  			goto e_inval;
>>>>  
>>>>  		/* hop-by-hop / destination options are privileged option */
>>>> -		retv = -EPERM;
>>>> -		if (optname != IPV6_RTHDR && !capable(CAP_NET_RAW))
>>>> +		if (optname != IPV6_RTHDR && !capable(CAP_NET_RAW)) {
>>>> +			retv = -EPERM;
>>>>  			break;
>>>> +		}
>>> I'm not sure you need this since it doesn't actually change anything,
>>> setting the error before the check just seems like the style in this
>>> function.  It will get set to -EINVAL below...
>>>
>> For uncommon expection, if the check is true, we set the error. 
>> it will reduce the number of setting retv value.
> 
> Do you have some emotional attachment to this change? :-/
> 
> The compiler optimizer will make the transformation you claim is
> now possible, all by itself, if it is deemed more optimal.
> 
> Never make any claims about how many times something will or will not
> happen in the compiled output without consulting and providing
> examples of the resulting assembler.  Otherwise you are just guessing.
> 
> This is just a pointless change, and as Brian said it disagrees with
> the way this construct is made in the rest of this function.  So
> please take it out of your patch.
sorry for repling late.
maybe what i describe is not explicit.

in this function the style is like this:
case XX:
{
  retv = -EPROTO;
  if (condition1) break;
  ...
  retv = -EFAULT;
  if (condition2) break;
  ...
  retv = -EINVAL;
  if (condition3) break;
  ...
}
when the conditions are all false, the retv value is set three times.

but with the following style, the retv value is not set.
case XX:
{
  if (condition1){ retv = -EPROTO;break;}
  ...
  if (condition2){ retv = -EFAULT;break;}
  ...
  if (condition3){ retv = -EINVAL;break;}
  ...
}
Seting the retv value when expection is occurred almost don't improve
the performance, but it's a little better.