[PATCH net-next 0/6] net_sched: clean up some chekpatch errors

[PATCH net-next 0/6] net_sched: clean up some chekpatch errors

[Yang Yingliang]

Clean up some checkpatch errors in "net/sched/*"

Yang Yingliang (6):
  net_sched: remove unnecessary parentheses while return
  net_sched: cls_bpf: use tabs to do indent
  net_sched: change "foo* bar" to "foo *bar"
  net_sched: add space around '>' and before '('
  net_sched: sfq: put sfq_unlink in a do - while loop
  net_sched: make macro be enclosed in parenthesis

 net/sched/act_api.c     |  3 ++-
 net/sched/cls_bpf.c     |  2 +-
 net/sched/cls_u32.c     |  2 +-
 net/sched/em_meta.c     |  4 +++-
 net/sched/sch_cbq.c     |  3 ++-
 net/sched/sch_dsmark.c  |  2 +-
 net/sched/sch_generic.c |  4 ++--
 net/sched/sch_netem.c   |  2 +-
 net/sched/sch_sfq.c     | 10 ++++++----
 9 files changed, 19 insertions(+), 13 deletions(-)

-- 
1.8.0

[PATCH net-next 1/6] net_sched: remove unnecessary parentheses while return

[Yang Yingliang]

return is not a function, parentheses are not required.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/act_api.c    | 3 ++-
 net/sched/sch_dsmark.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index fd70728..92a3c31 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -191,7 +191,8 @@ u32 tcf_hash_new_index(u32 *idx_gen, struct tcf_hashinfo *hinfo)
 			val = 1;
 	} while (tcf_hash_lookup(val, hinfo));
 
-	return (*idx_gen = val);
+	*idx_gen = val;
+	return val;
 }
 EXPORT_SYMBOL(tcf_hash_new_index);
 
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 3886365..0952fd2 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -47,7 +47,7 @@ struct dsmark_qdisc_data {
 
 static inline int dsmark_valid_index(struct dsmark_qdisc_data *p, u16 index)
 {
-	return (index <= p->indices && index > 0);
+	return index <= p->indices && index > 0;
 }
 
 /* ------------------------- Class/flow operations ------------------------- */
-- 
1.8.0

[PATCH net-next 2/6] net_sched: cls_bpf: use tabs to do indent

[Yang Yingliang]

Code indent should use tabs where possible

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/cls_bpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 1002a82..d7c72be 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -323,7 +323,7 @@ static int cls_bpf_dump(struct tcf_proto *tp, unsigned long fh,
 	if (nla == NULL)
 		goto nla_put_failure;
 
-        memcpy(nla_data(nla), prog->bpf_ops, nla_len(nla));
+	memcpy(nla_data(nla), prog->bpf_ops, nla_len(nla));
 
 	if (tcf_exts_dump(skb, &prog->exts, &bpf_ext_map) < 0)
 		goto nla_put_failure;
-- 
1.8.0

[PATCH net-next 3/6] net_sched: change "foo* bar" to "foo *bar"

[Yang Yingliang]

"foo* bar" or "foo * bar" should be "foo *bar".

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/cls_u32.c     | 2 +-
 net/sched/sch_generic.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index eb07a1e..59e546c 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -352,7 +352,7 @@ static int u32_destroy_key(struct tcf_proto *tp, struct tc_u_knode *n)
 	return 0;
 }
 
-static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode* key)
+static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
 {
 	struct tc_u_knode **kp;
 	struct tc_u_hnode *ht = key->ht_up;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 922a094..6a91d7d 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -338,13 +338,13 @@ EXPORT_SYMBOL(netif_carrier_off);
    cheaper.
  */
 
-static int noop_enqueue(struct sk_buff *skb, struct Qdisc * qdisc)
+static int noop_enqueue(struct sk_buff *skb, struct Qdisc *qdisc)
 {
 	kfree_skb(skb);
 	return NET_XMIT_CN;
 }
 
-static struct sk_buff *noop_dequeue(struct Qdisc * qdisc)
+static struct sk_buff *noop_dequeue(struct Qdisc *qdisc)
 {
 	return NULL;
 }
-- 
1.8.0

[PATCH net-next 4/6] net_sched: add space around '>' and before '('

[Yang Yingliang]

Spaces required around that '>' (ctx:VxV) and
before the open parenthesis '('.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/sch_cbq.c   | 3 ++-
 net/sched/sch_netem.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 7a42c81..d5a8a4b 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1058,7 +1058,8 @@ static void cbq_normalize_quanta(struct cbq_sched_data *q, int prio)
 				cl->quantum = (cl->weight*cl->allot*q->nclasses[prio])/
 					q->quanta[prio];
 			}
-			if (cl->quantum <= 0 || cl->quantum>32*qdisc_dev(cl->qdisc)->mtu) {
+			if (cl->quantum <= 0 ||
+			    cl->quantum > 32*qdisc_dev(cl->qdisc)->mtu) {
 				pr_warning("CBQ: class %08x has bad quantum==%ld, repaired.\n",
 					   cl->common.classid, cl->quantum);
 				cl->quantum = qdisc_dev(cl->qdisc)->mtu/2 + 1;
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index bccd52b..f3befd6 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -729,7 +729,7 @@ static int get_loss_clg(struct Qdisc *sch, const struct nlattr *attr)
 	nla_for_each_nested(la, attr, rem) {
 		u16 type = nla_type(la);
 
-		switch(type) {
+		switch (type) {
 		case NETEM_LOSS_GI: {
 			const struct tc_netem_gimodel *gi = nla_data(la);
 
-- 
1.8.0

[PATCH net-next 5/6] net_sched: sfq: put sfq_unlink in a do - while loop

[Yang Yingliang]

Macros with multiple statements should be enclosed in a do - while loop

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/sch_sfq.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index d3a1bc2..76f01e0 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -237,10 +237,12 @@ static inline void sfq_link(struct sfq_sched_data *q, sfq_index x)
 }
 
 #define sfq_unlink(q, x, n, p)			\
-	n = q->slots[x].dep.next;		\
-	p = q->slots[x].dep.prev;		\
-	sfq_dep_head(q, p)->next = n;		\
-	sfq_dep_head(q, n)->prev = p
+	do {					\
+		n = q->slots[x].dep.next;	\
+		p = q->slots[x].dep.prev;	\
+		sfq_dep_head(q, p)->next = n;	\
+		sfq_dep_head(q, n)->prev = p;	\
+	} while (0)
 
 
 static inline void sfq_dec(struct sfq_sched_data *q, sfq_index x)
-- 
1.8.0

[PATCH net-next 6/6] net_sched: make macro be enclosed in parenthesis

[Yang Yingliang]

Macros with complex values should be enclosed in parenthesis

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/em_meta.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
index e5cef956..852cd62 100644
--- a/net/sched/em_meta.c
+++ b/net/sched/em_meta.c
@@ -272,10 +272,12 @@ META_COLLECTOR(int_rtiif)
  **************************************************************************/
 
 #define SKIP_NONLOCAL(skb)			\
+({						\
 	if (unlikely(skb->sk == NULL)) {	\
 		*err = -1;			\
 		return;				\
-	}
+	}					\
+})
 
 META_COLLECTOR(int_sk_family)
 {
-- 
1.8.0

RE: [PATCH net-next 6/6] net_sched: make macro be enclosed in parenthesis

[David Laight]

>  #define SKIP_NONLOCAL(skb)			\
> +({						\
>  	if (unlikely(skb->sk == NULL)) {	\
>  		*err = -1;			\
>  		return;				\
> -	}
> +	}					\
> +})

That one should be lined up against the fence and shot :-)

Re: [PATCH net-next 6/6] net_sched: make macro be enclosed in parenthesis

[Yang Yingliang]

On 2013/12/10 21:01, David Laight wrote:
>>  #define SKIP_NONLOCAL(skb)			\
>> +({						\
>>  	if (unlikely(skb->sk == NULL)) {	\
>>  		*err = -1;			\
>>  		return;				\
>> -	}
>> +	}					\
>> +})
> 
> That one should be lined up against the fence and shot :-)
> 
> 

Hmm, not fully understand, did you mean that:

#define SKIP_NONLOCAL(skb) ({			\
 	if (unlikely(skb->sk == NULL)) {	\
 		*err = -1;			\
		return;				\
	}					\
})

Re: [PATCH net-next 1/6] net_sched: remove unnecessary parentheses while return

[David Miller]

From: Yang Yingliang <yangyingliang@huawei.com>
Date: Tue, 10 Dec 2013 20:55:29 +0800

> return is not a function, parentheses are not required.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

Applied.

Re: [PATCH net-next 2/6] net_sched: cls_bpf: use tabs to do indent

[David Miller]

From: Yang Yingliang <yangyingliang@huawei.com>
Date: Tue, 10 Dec 2013 20:55:30 +0800

> Code indent should use tabs where possible
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

Applied.

Re: [PATCH net-next 3/6] net_sched: change "foo* bar" to "foo *bar"

[David Miller]

From: Yang Yingliang <yangyingliang@huawei.com>
Date: Tue, 10 Dec 2013 20:55:31 +0800

> "foo* bar" or "foo * bar" should be "foo *bar".
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

Applied.

Re: [PATCH net-next 4/6] net_sched: add space around '>' and before '('

[David Miller]

From: Yang Yingliang <yangyingliang@huawei.com>
Date: Tue, 10 Dec 2013 20:55:32 +0800

> Spaces required around that '>' (ctx:VxV) and
> before the open parenthesis '('.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

Applied.

Re: [PATCH net-next 5/6] net_sched: sfq: put sfq_unlink in a do - while loop

[David Miller]

From: Yang Yingliang <yangyingliang@huawei.com>
Date: Tue, 10 Dec 2013 20:55:33 +0800

> Macros with multiple statements should be enclosed in a do - while loop
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

Applied.

Re: [PATCH net-next 6/6] net_sched: make macro be enclosed in parenthesis

[David Miller]

From: Yang Yingliang <yangyingliang@huawei.com>
Date: Tue, 10 Dec 2013 20:55:34 +0800

> Macros with complex values should be enclosed in parenthesis
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  net/sched/em_meta.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
> index e5cef956..852cd62 100644
> --- a/net/sched/em_meta.c
> +++ b/net/sched/em_meta.c
> @@ -272,10 +272,12 @@ META_COLLECTOR(int_rtiif)
>   **************************************************************************/
>  
>  #define SKIP_NONLOCAL(skb)			\
> +({						\
>  	if (unlikely(skb->sk == NULL)) {	\
>  		*err = -1;			\
>  		return;				\
> -	}
> +	}					\
> +})

I can't apply this.

First of all, "({ })" is for statements that evaluate to an lvalue,
this macro does not.

Second of all, and more importantly, this macro needs to be eliminated
entirely.  It completely hides control flow, and in the past we've
killed macros which do this such as the old netlink attribute
builders.

The control flow should be inlined and expanded explicitly in the
code so that someone who reads it can tell the control flow can
be changed by the statement.  Compare:

	SKIP_NONLOCAL(skb)

to:

	if (skip_nonlocal(skb)) {
		*err = -1;
		return;
	}

[PATCH net-next] net_sched: expand control flow of macro SKIP_NONLOCAL

[Yang Yingliang]

From: Yang Yingliang <yangyingliang@huawei.com>

SKIP_NONLOCAL hides the control flow. The control flow should be
inlined and expanded explicitly in code so that someone who reads
it can tell the control flow can be changed by the statement.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 net/sched/em_meta.c | 157 ++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 122 insertions(+), 35 deletions(-)

diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
index e5cef956..382519a 100644
--- a/net/sched/em_meta.c
+++ b/net/sched/em_meta.c
@@ -271,40 +271,52 @@ META_COLLECTOR(int_rtiif)
  * Socket Attributes
  **************************************************************************/
 
-#define SKIP_NONLOCAL(skb)			\
-	if (unlikely(skb->sk == NULL)) {	\
-		*err = -1;			\
-		return;				\
-	}
+#define skip_nonlocal(skb) \
+	(unlikely(skb->sk == NULL))
 
 META_COLLECTOR(int_sk_family)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_family;
 }
 
 META_COLLECTOR(int_sk_state)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_state;
 }
 
 META_COLLECTOR(int_sk_reuse)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_reuse;
 }
 
 META_COLLECTOR(int_sk_bound_if)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	/* No error if bound_dev_if is 0, legal userspace check */
 	dst->value = skb->sk->sk_bound_dev_if;
 }
 
 META_COLLECTOR(var_sk_bound_if)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 
 	if (skb->sk->sk_bound_dev_if == 0) {
 		dst->value = (unsigned long) "any";
@@ -322,151 +334,226 @@ META_COLLECTOR(var_sk_bound_if)
 
 META_COLLECTOR(int_sk_refcnt)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = atomic_read(&skb->sk->sk_refcnt);
 }
 
 META_COLLECTOR(int_sk_rcvbuf)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_rcvbuf;
 }
 
 META_COLLECTOR(int_sk_shutdown)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_shutdown;
 }
 
 META_COLLECTOR(int_sk_proto)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_protocol;
 }
 
 META_COLLECTOR(int_sk_type)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_type;
 }
 
 META_COLLECTOR(int_sk_rmem_alloc)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = sk_rmem_alloc_get(skb->sk);
 }
 
 META_COLLECTOR(int_sk_wmem_alloc)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = sk_wmem_alloc_get(skb->sk);
 }
 
 META_COLLECTOR(int_sk_omem_alloc)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = atomic_read(&skb->sk->sk_omem_alloc);
 }
 
 META_COLLECTOR(int_sk_rcv_qlen)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_receive_queue.qlen;
 }
 
 META_COLLECTOR(int_sk_snd_qlen)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_write_queue.qlen;
 }
 
 META_COLLECTOR(int_sk_wmem_queued)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_wmem_queued;
 }
 
 META_COLLECTOR(int_sk_fwd_alloc)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_forward_alloc;
 }
 
 META_COLLECTOR(int_sk_sndbuf)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_sndbuf;
 }
 
 META_COLLECTOR(int_sk_alloc)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = (__force int) skb->sk->sk_allocation;
 }
 
 META_COLLECTOR(int_sk_hash)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_hash;
 }
 
 META_COLLECTOR(int_sk_lingertime)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_lingertime / HZ;
 }
 
 META_COLLECTOR(int_sk_err_qlen)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_error_queue.qlen;
 }
 
 META_COLLECTOR(int_sk_ack_bl)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_ack_backlog;
 }
 
 META_COLLECTOR(int_sk_max_ack_bl)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_max_ack_backlog;
 }
 
 META_COLLECTOR(int_sk_prio)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_priority;
 }
 
 META_COLLECTOR(int_sk_rcvlowat)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_rcvlowat;
 }
 
 META_COLLECTOR(int_sk_rcvtimeo)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_rcvtimeo / HZ;
 }
 
 META_COLLECTOR(int_sk_sndtimeo)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_sndtimeo / HZ;
 }
 
 META_COLLECTOR(int_sk_sendmsg_off)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_frag.offset;
 }
 
 META_COLLECTOR(int_sk_write_pend)
 {
-	SKIP_NONLOCAL(skb);
+	if (skip_nonlocal(skb)) {
+		*err = -1;
+		return;
+	}
 	dst->value = skb->sk->sk_write_pending;
 }
 
-- 1.8.0

RE: [PATCH net-next 6/6] net_sched: make macro be enclosed in parenthesis

[David Laight]

> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of David Miller
> From: Yang Yingliang <yangyingliang@huawei.com>
> Date: Tue, 10 Dec 2013 20:55:34 +0800
> 
> > Macros with complex values should be enclosed in parenthesis
> >
> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> > ---
> >  net/sched/em_meta.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
> > index e5cef956..852cd62 100644
> > --- a/net/sched/em_meta.c
> > +++ b/net/sched/em_meta.c
> > @@ -272,10 +272,12 @@ META_COLLECTOR(int_rtiif)
> >   **************************************************************************/
> >
> >  #define SKIP_NONLOCAL(skb)			\
> > +({						\
> >  	if (unlikely(skb->sk == NULL)) {	\
> >  		*err = -1;			\
> >  		return;				\
> > -	}
> > +	}					\
> > +})
> 
> I can't apply this.
> 
> First of all, "({ })" is for statements that evaluate to an lvalue,
> this macro does not.
> 
> Second of all, and more importantly, this macro needs to be eliminated
> entirely.  It completely hides control flow, and in the past we've
> killed macros which do this such as the old netlink attribute
> builders.
> 
> The control flow should be inlined and expanded explicitly in the
> code so that someone who reads it can tell the control flow can
> be changed by the statement.  Compare:
> 
> 	SKIP_NONLOCAL(skb)
> 
> to:
> 
> 	if (skip_nonlocal(skb)) {
> 		*err = -1;
> 		return;
> 	}

Or:
	if (!skb->sk)
		goto no_sk;
	...
	return;

no_sk:
	*err = -1;
	return;

Actually WTF is a void function doing having an 'int *err' argument?
These should just be:
	if (!skb->sk)
		return -1;

	David

Re: [PATCH net-next] net_sched: expand control flow of macro SKIP_NONLOCAL

[David Miller]

From: Yang Yingliang <yangyingliang@huawei.com>
Date: Wed, 11 Dec 2013 15:17:11 +0800

> From: Yang Yingliang <yangyingliang@huawei.com>
> 
> SKIP_NONLOCAL hides the control flow. The control flow should be
> inlined and expanded explicitly in code so that someone who reads
> it can tell the control flow can be changed by the statement.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

This looks better, applied, thanks.