diff mbox series

net/ipv6: remove redundant err variable

Message ID 20220112082655.667680-1-chi.minghao@zte.com.cn (mailing list archive)
State Rejected
Delegated to: Netdev Maintainers
Headers show
Series net/ipv6: remove redundant err variable | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 15 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

CGEL Jan. 12, 2022, 8:26 a.m. UTC 
From: Minghao Chi <chi.minghao@zte.com.cn>

Return value from ip6_tnl_err() directly instead
of taking this in another redundant variable.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
Signed-off-by: CGEL ZTE <cgel.zte@gmail.com>
---
 net/ipv6/ip6_tunnel.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

David Ahern Jan. 12, 2022, 3:54 p.m. UTC | #1 
On 1/12/22 1:26 AM, cgel.zte@gmail.com wrote:
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index fe786df4f849..897194eb3b89 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -698,13 +698,12 @@ mplsip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  	    u8 type, u8 code, int offset, __be32 info)
>  {
>  	__u32 rel_info = ntohl(info);
> -	int err, rel_msg = 0;
> +	int rel_msg = 0;

this line needs to be moved down to maintain reverse xmas tree ordering.

You need to make the subject
'[PATCH net-next] net/ipv6: remove redundant err variable'

and since net-next is closed you will need to resubmit when it is open.

>  	u8 rel_type = type;
>  	u8 rel_code = code;
>  
> -	err = ip6_tnl_err(skb, IPPROTO_MPLS, opt, &rel_type, &rel_code,
> +	return ip6_tnl_err(skb, IPPROTO_MPLS, opt, &rel_type, &rel_code,
>  			  &rel_msg, &rel_info, offset);
> -	return err;
>  }
>  
>  static int ip4ip6_dscp_ecn_decapsulate(const struct ip6_tnl *t,
Jakub Kicinski Jan. 12, 2022, 4:39 p.m. UTC | #2 
On Wed, 12 Jan 2022 08:54:41 -0700 David Ahern wrote:
> On 1/12/22 1:26 AM, cgel.zte@gmail.com wrote:
> > diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> > index fe786df4f849..897194eb3b89 100644
> > --- a/net/ipv6/ip6_tunnel.c
> > +++ b/net/ipv6/ip6_tunnel.c
> > @@ -698,13 +698,12 @@ mplsip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> >  	    u8 type, u8 code, int offset, __be32 info)
> >  {
> >  	__u32 rel_info = ntohl(info);
> > -	int err, rel_msg = 0;
> > +	int rel_msg = 0;  
> 
> this line needs to be moved down to maintain reverse xmas tree ordering.
> 
> You need to make the subject
> '[PATCH net-next] net/ipv6: remove redundant err variable'
> 
> and since net-next is closed you will need to resubmit when it is open.

Frankly I'm not sure these type of patches make sense.

Minghao how many instances of this pattern are there in the tree today?

What tools can we use to ensure the pattern does not get added back in
new patches?

> >  	u8 rel_type = type;
> >  	u8 rel_code = code;
> >  
> > -	err = ip6_tnl_err(skb, IPPROTO_MPLS, opt, &rel_type, &rel_code,
> > +	return ip6_tnl_err(skb, IPPROTO_MPLS, opt, &rel_type, &rel_code,
> >  			  &rel_msg, &rel_info, offset);

You should re-align continuation lines when you move the opening
bracket.

> > -	return err;
> >  }
CGEL Jan. 19, 2022, 3 a.m. UTC | #3 
In the bot, I wrote a coccinelle rule myself to search for such problems,
maybe use the rules to scan before submitting the patch.
Jakub Kicinski Jan. 19, 2022, 3:28 a.m. UTC | #4 
On Wed, 19 Jan 2022 03:00:03 +0000 cgel.zte@gmail.com wrote:
> In the bot, I wrote a coccinelle rule myself to search for such problems,
> maybe use the rules to scan before submitting the patch.

Is it

scripts/coccinelle/misc/returnvar.cocci

? 

How many of such patches do you have pending?
CGEL Jan. 19, 2022, 5:55 a.m. UTC | #5 
There are more than 3,000 numbers retrieved by the rules I wrote.
Of course, many of these 3,000 are false positives, and maybe 
there are 300 patches.I wrote this rule similar to returnvar.cocci.
Jakub Kicinski Jan. 19, 2022, 3:59 p.m. UTC | #6 
On Wed, 19 Jan 2022 05:55:57 +0000 cgel.zte@gmail.com wrote:
> There are more than 3,000 numbers retrieved by the rules I wrote.
> Of course, many of these 3,000 are false positives, and maybe 
> there are 300 patches.I wrote this rule similar to returnvar.cocci.

Can you start by improving returnvar.cocci so it catches all the cases?
Then make sure your patches build and don't introduce formatting
problems. Then post them.
diff mbox series

Patch

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index fe786df4f849..897194eb3b89 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -698,13 +698,12 @@  mplsip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	    u8 type, u8 code, int offset, __be32 info)
 {
 	__u32 rel_info = ntohl(info);
-	int err, rel_msg = 0;
+	int rel_msg = 0;
 	u8 rel_type = type;
 	u8 rel_code = code;
 
-	err = ip6_tnl_err(skb, IPPROTO_MPLS, opt, &rel_type, &rel_code,
+	return ip6_tnl_err(skb, IPPROTO_MPLS, opt, &rel_type, &rel_code,
 			  &rel_msg, &rel_info, offset);
-	return err;
 }
 
 static int ip4ip6_dscp_ecn_decapsulate(const struct ip6_tnl *t,