[net-next,v3] net: assign err to 0 at begin in do_setlink() function

From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>

each netlink attribute have proper process when error happen,
when exit one attribute process, it implies that no error,
so err = 0; is useless.

assign err = 0; at beginning if all attributes not set.

v1 -> v2:
	fix review comment from David, clear err before
	nla_for_each_nested()

v2 -> v3:
	maybe wrong understanding of David comment,
	provide a new version

Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
---
 net/core/rtnetlink.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

From: yuan linyu <cugyly@163.com>
Date: Thu, 16 Nov 2017 19:59:48 +0800

> From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
> 
> each netlink attribute have proper process when error happen,
> when exit one attribute process, it implies that no error,
> so err = 0; is useless.
> 
> assign err = 0; at beginning if all attributes not set.
> 
> v1 -> v2:
> 	fix review comment from David, clear err before
> 	nla_for_each_nested()
> 
> v2 -> v3:
> 	maybe wrong understanding of David comment,
> 	provide a new version
> 
> Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>

I'm sorry I still find it hard to accept this change.

What about all of the assignments of 'err' which only branch to
'errout' if err is negative?  It is not easy to see that none of those
case ever result in 'err' holding a positive non-zero value.

The code as-is is the easiest to understand, audit and prove correct
in the error-free case.  And this because of the explicit clearing or
'err' to zero late in the function.

Thanks you.

On 五, 2017-11-17 at 15:08 +0900, David Miller wrote:
> From: yuan linyu <cugyly@163.com>
> Date: Thu, 16 Nov 2017 19:59:48 +0800
> 
> > From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
> > 
> > each netlink attribute have proper process when error happen,
> > when exit one attribute process, it implies that no error,
> > so err = 0; is useless.
> > 
> > assign err = 0; at beginning if all attributes not set.
> > 
> > v1 -> v2:
> >       fix review comment from David, clear err before
> >       nla_for_each_nested()
> > 
> > v2 -> v3:
> >       maybe wrong understanding of David comment,
> >       provide a new version
> > 
> > Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
> 
> I'm sorry I still find it hard to accept this change.
> 
> What about all of the assignments of 'err' which only branch to
> 'errout' if err is negative?  It is not easy to see that none of those
> case ever result in 'err' holding a positive non-zero value.
yes, i also try to check any function return > 0, but it hard,
maybe some function not follow common rule.
> 
> The code as-is is the easiest to understand, audit and prove correct
> in the error-free case.  And this because of the explicit clearing or
> 'err' to zero late in the function.
yes, but this clearing will do three times every function call,
but it is not big issue 
> 
> Thanks you.