diff mbox series

[net,1/5] net sched actions: fix coding style in pedit action

Message ID 1529427368-17129-2-git-send-email-mrv@mojatatu.com
State Deferred, archived
Delegated to: David Miller
Headers show
Series net sched actions: code style cleanup and fixes | expand

Commit Message

Roman Mashak June 19, 2018, 4:56 p.m. UTC 
Fix coding style issues in tc pedit action detected by the
checkpatch script.

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 net/sched/act_pedit.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

Comments

Davide Caratti June 20, 2018, 12:29 p.m. UTC | #1 
On Tue, 2018-06-19 at 12:56 -0400, Roman Mashak wrote:
> Fix coding style issues in tc pedit action detected by the
> checkpatch script.
> 
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
...

> ---
> @@ -316,16 +318,15 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
>  						hoffset + tkey->at);
>  					goto bad;
>  				}
> -				d = skb_header_pointer(skb, hoffset + tkey->at, 1,
> -						       &_d);
> +				d = skb_header_pointer(skb, hoffset + tkey->at,
> +						       1, &_d);
> 				if (!d)
>  					goto bad;
>  				offset += (*d & tkey->offmask) >> tkey->shift;
>  			}

hello Roman,

nit: while we are here, what about changing the declaration of _d and *d
to u8, so that the bitwise operation is done on unsigned?

BTW: the patch (and the series) looks ok, but I guess it will better
target net-next when the branch reopens

thanks!
Roman Mashak June 20, 2018, 1:25 p.m. UTC | #2 
Davide Caratti <dcaratti@redhat.com> writes:

> On Tue, 2018-06-19 at 12:56 -0400, Roman Mashak wrote:
>> Fix coding style issues in tc pedit action detected by the
>> checkpatch script.
>> 
>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ...
>
>> ---
>> @@ -316,16 +318,15 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
>>  						hoffset + tkey->at);
>>  					goto bad;
>>  				}
>> -				d = skb_header_pointer(skb, hoffset + tkey->at, 1,
>> -						       &_d);
>> +				d = skb_header_pointer(skb, hoffset + tkey->at,
>> +						       1, &_d);
>> 				if (!d)
>>  					goto bad;
>>  				offset += (*d & tkey->offmask) >> tkey->shift;
>>  			}
>
> hello Roman,
>
> nit: while we are here, what about changing the declaration of _d and *d
> to u8, so that the bitwise operation is done on unsigned?

Yes makes sense, I will send v2 in net-next once opened. Thanks Davide.

> BTW: the patch (and the series) looks ok, but I guess it will better
> target net-next when the branch reopens
diff mbox series

Patch

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 8a925c72db5f..e4b29ee79ba8 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -136,15 +136,15 @@  static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 {
 	struct tc_action_net *tn = net_generic(net, pedit_net_id);
 	struct nlattr *tb[TCA_PEDIT_MAX + 1];
-	struct nlattr *pattr;
-	struct tc_pedit *parm;
-	int ret = 0, err;
-	struct tcf_pedit *p;
 	struct tc_pedit_key *keys = NULL;
 	struct tcf_pedit_key_ex *keys_ex;
+	struct tc_pedit *parm;
+	struct nlattr *pattr;
+	struct tcf_pedit *p;
+	int ret = 0, err;
 	int ksize;
 
-	if (nla == NULL)
+	if (!nla)
 		return -EINVAL;
 
 	err = nla_parse_nested(tb, TCA_PEDIT_MAX, nla, pedit_policy, NULL);
@@ -175,7 +175,7 @@  static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 			return ret;
 		p = to_pedit(*a);
 		keys = kmalloc(ksize, GFP_KERNEL);
-		if (keys == NULL) {
+		if (!keys) {
 			tcf_idr_release(*a, bind);
 			kfree(keys_ex);
 			return -ENOMEM;
@@ -220,6 +220,7 @@  static void tcf_pedit_cleanup(struct tc_action *a)
 {
 	struct tcf_pedit *p = to_pedit(a);
 	struct tc_pedit_key *keys = p->tcfp_keys;
+
 	kfree(keys);
 	kfree(p->tcfp_keys_ex);
 }
@@ -284,7 +285,8 @@  static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 	if (p->tcfp_nkeys > 0) {
 		struct tc_pedit_key *tkey = p->tcfp_keys;
 		struct tcf_pedit_key_ex *tkey_ex = p->tcfp_keys_ex;
-		enum pedit_header_type htype = TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
+		enum pedit_header_type htype =
+			TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
 		enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
 
 		for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
@@ -316,16 +318,15 @@  static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 						hoffset + tkey->at);
 					goto bad;
 				}
-				d = skb_header_pointer(skb, hoffset + tkey->at, 1,
-						       &_d);
+				d = skb_header_pointer(skb, hoffset + tkey->at,
+						       1, &_d);
 				if (!d)
 					goto bad;
 				offset += (*d & tkey->offmask) >> tkey->shift;
 			}
 
 			if (offset % 4) {
-				pr_info("tc filter pedit"
-					" offset must be on 32 bit boundaries\n");
+				pr_info("tc filter pedit offset must be on 32 bit boundaries\n");
 				goto bad;
 			}
 
@@ -335,7 +336,8 @@  static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 				goto bad;
 			}
 
-			ptr = skb_header_pointer(skb, hoffset + offset, 4, &_data);
+			ptr = skb_header_pointer(skb, hoffset + offset,
+						 4, &_data);
 			if (!ptr)
 				goto bad;
 			/* just do it, baby */
@@ -358,8 +360,9 @@  static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 		}
 
 		goto done;
-	} else
+	} else {
 		WARN(1, "pedit BUG: index %d\n", p->tcf_index);
+	}
 
 bad:
 	p->tcf_qstats.overlimits++;