Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

[kbuild test robot]

Hi zhong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/zhong-jiang/net-ipv4-Use-BUG_ON-directly-instead-of-a-if-condition-followed-by-BUG/20180911-034152
config: x86_64-randconfig-x009-201836 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net//ipv4/tcp_input.c: In function 'tcp_collapse':
>> net//ipv4/tcp_input.c:4925:35: error: macro "BUG" passed 1 arguments, but takes just 0
            skb_put(nskb, size), size));
                                      ^
>> net//ipv4/tcp_input.c:4924:5: error: 'BUG' undeclared (first use in this function)
        BUG(skb_copy_bits(skb, offset,
        ^~~
   net//ipv4/tcp_input.c:4924:5: note: each undeclared identifier is reported only once for each function it appears in
   net//ipv4/tcp_input.c: In function 'tcp_urg':
   net//ipv4/tcp_input.c:5318:40: error: macro "BUG" passed 1 arguments, but takes just 0
       BUG(skb_copy_bits(skb, ptr, &tmp, 1));
                                           ^
   net//ipv4/tcp_input.c:5318:4: error: 'BUG' undeclared (first use in this function)
       BUG(skb_copy_bits(skb, ptr, &tmp, 1));
       ^~~

vim +/BUG +4925 net//ipv4/tcp_input.c

  4838	
  4839	/* Collapse contiguous sequence of skbs head..tail with
  4840	 * sequence numbers start..end.
  4841	 *
  4842	 * If tail is NULL, this means until the end of the queue.
  4843	 *
  4844	 * Segments with FIN/SYN are not collapsed (only because this
  4845	 * simplifies code)
  4846	 */
  4847	static void
  4848	tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
  4849		     struct sk_buff *head, struct sk_buff *tail, u32 start, u32 end)
  4850	{
  4851		struct sk_buff *skb = head, *n;
  4852		struct sk_buff_head tmp;
  4853		bool end_of_skbs;
  4854	
  4855		/* First, check that queue is collapsible and find
  4856		 * the point where collapsing can be useful.
  4857		 */
  4858	restart:
  4859		for (end_of_skbs = true; skb != NULL && skb != tail; skb = n) {
  4860			n = tcp_skb_next(skb, list);
  4861	
  4862			/* No new bits? It is possible on ofo queue. */
  4863			if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
  4864				skb = tcp_collapse_one(sk, skb, list, root);
  4865				if (!skb)
  4866					break;
  4867				goto restart;
  4868			}
  4869	
  4870			/* The first skb to collapse is:
  4871			 * - not SYN/FIN and
  4872			 * - bloated or contains data before "start" or
  4873			 *   overlaps to the next one.
  4874			 */
  4875			if (!(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
  4876			    (tcp_win_from_space(sk, skb->truesize) > skb->len ||
  4877			     before(TCP_SKB_CB(skb)->seq, start))) {
  4878				end_of_skbs = false;
  4879				break;
  4880			}
  4881	
  4882			if (n && n != tail &&
  4883			    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
  4884				end_of_skbs = false;
  4885				break;
  4886			}
  4887	
  4888			/* Decided to skip this, advance start seq. */
  4889			start = TCP_SKB_CB(skb)->end_seq;
  4890		}
  4891		if (end_of_skbs ||
  4892		    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
  4893			return;
  4894	
  4895		__skb_queue_head_init(&tmp);
  4896	
  4897		while (before(start, end)) {
  4898			int copy = min_t(int, SKB_MAX_ORDER(0, 0), end - start);
  4899			struct sk_buff *nskb;
  4900	
  4901			nskb = alloc_skb(copy, GFP_ATOMIC);
  4902			if (!nskb)
  4903				break;
  4904	
  4905			memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
  4906	#ifdef CONFIG_TLS_DEVICE
  4907			nskb->decrypted = skb->decrypted;
  4908	#endif
  4909			TCP_SKB_CB(nskb)->seq = TCP_SKB_CB(nskb)->end_seq = start;
  4910			if (list)
  4911				__skb_queue_before(list, skb, nskb);
  4912			else
  4913				__skb_queue_tail(&tmp, nskb); /* defer rbtree insertion */
  4914			skb_set_owner_r(nskb, sk);
  4915	
  4916			/* Copy data, releasing collapsed skbs. */
  4917			while (copy > 0) {
  4918				int offset = start - TCP_SKB_CB(skb)->seq;
  4919				int size = TCP_SKB_CB(skb)->end_seq - start;
  4920	
  4921				BUG_ON(offset < 0);
  4922				if (size > 0) {
  4923					size = min(copy, size);
> 4924					BUG(skb_copy_bits(skb, offset,
> 4925							  skb_put(nskb, size), size));
  4926					TCP_SKB_CB(nskb)->end_seq += size;
  4927					copy -= size;
  4928					start += size;
  4929				}
  4930				if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
  4931					skb = tcp_collapse_one(sk, skb, list, root);
  4932					if (!skb ||
  4933					    skb == tail ||
  4934					    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
  4935						goto end;
  4936	#ifdef CONFIG_TLS_DEVICE
  4937					if (skb->decrypted != nskb->decrypted)
  4938						goto end;
  4939	#endif
  4940				}
  4941			}
  4942		}
  4943	end:
  4944		skb_queue_walk_safe(&tmp, skb, n)
  4945			tcp_rbtree_insert(root, skb);
  4946	}
  4947	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Attachment: .config.gz
Description: application/gzip