Re: [PATCH] usb: gadget: eem: Use common error handling code in eem_unwrap()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on usb/usb-testing]
> [also build test ERROR on usb/usb-next usb/usb-linus westeri-thunderbolt/next linus/master v6.11 next-20240925]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Elfring/usb-gadget-eem-Use-common-error-handling-code-in-eem_unwrap/20240925-233931
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> patch link:    https://lore.kernel.org/r/59fadd5a-6574-4379-98ac-cc4f11b675cc%40web.de
> patch subject: [PATCH] usb: gadget: eem: Use common error handling code in eem_unwrap()
> config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240926/202409261347.SAA10KGy-lkp@xxxxxxxxx/config)
> compiler: alpha-linux-gcc (GCC) 13.3.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240926/202409261347.SAA10KGy-lkp@xxxxxxxxx/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202409261347.SAA10KGy-lkp@xxxxxxxxx/
>
> All errors (new ones prefixed by >>):
>
>    drivers/usb/gadget/function/f_eem.c: In function 'eem_unwrap':
>>> drivers/usb/gadget/function/f_eem.c:547:29: error: 'ep' undeclared (first use in this function); did you mean 'up'?
>      547 |         usb_ep_free_request(ep, req);
>          |                             ^~
>          |                             up
>    drivers/usb/gadget/function/f_eem.c:547:29: note: each undeclared identifier is reported only once for each function it appears in
>>> drivers/usb/gadget/function/f_eem.c:547:33: error: 'req' undeclared (first use in this function); did you mean 'rq'?
>      547 |         usb_ep_free_request(ep, req);
>          |                                 ^~~
>          |                                 rq
>>> drivers/usb/gadget/function/f_eem.c:549:27: error: 'skb2' undeclared (first use in this function); did you mean 'skb'?
>      549 |         dev_kfree_skb_any(skb2);
>          |                           ^~~~
>          |                           skb

Is there development support to adjust the scope for mentioned local variables?


> vim +547 drivers/usb/gadget/function/f_eem.c
>
>    387
>    388	/*
>    389	 * Remove the EEM header.  Note that there can be many EEM packets in a single
>    390	 * USB transfer, so we need to break them out and handle them independently.
>    391	 */
>    392	static int eem_unwrap(struct gether *port,
>    393				struct sk_buff *skb,
>    394				struct sk_buff_head *list)
>    395	{
>    396		struct usb_composite_dev	*cdev = port->func.config->cdev;
>    397		int				status = 0;
>    398
>    399		do {
>    400			struct sk_buff	*skb2;
>    401			u16		header;
>    402			u16		len = 0;
>    403
>    404			if (skb->len < EEM_HLEN) {
>    405				status = -EINVAL;
>    406				DBG(cdev, "invalid EEM header\n");
>    407				goto error;
>    408			}
>    409
>    410			/* remove the EEM header */
>    411			header = get_unaligned_le16(skb->data);
>    412			skb_pull(skb, EEM_HLEN);
>    413
>    414			/* EEM packet header format:
>    415			 * b0..14:	EEM type dependent (data or command)
>    416			 * b15:		bmType (0 == data, 1 == command)
>    417			 */
>    418			if (header & BIT(15)) {
>    419				struct usb_request	*req;
>    420				struct in_context	*ctx;
>    421				struct usb_ep		*ep;
>    422				u16			bmEEMCmd;
>    423
>    424				/* EEM command packet format:
>    425				 * b0..10:	bmEEMCmdParam
>    426				 * b11..13:	bmEEMCmd
>    427				 * b14:		reserved (must be zero)
>    428				 * b15:		bmType (1 == command)
>    429				 */
>    430				if (header & BIT(14))
>    431					continue;
>    432
>    433				bmEEMCmd = (header >> 11) & 0x7;
>    434				switch (bmEEMCmd) {
>    435				case 0: /* echo */
>    436					len = header & 0x7FF;
>    437					if (skb->len < len) {
>    438						status = -EOVERFLOW;
>    439						goto error;
>    440					}
>    441
>    442					skb2 = skb_clone(skb, GFP_ATOMIC);
>    443					if (unlikely(!skb2)) {
>    444						DBG(cdev, "EEM echo response error\n");
>    445						goto next;
>    446					}
>    447					skb_trim(skb2, len);
>    448					put_unaligned_le16(BIT(15) | BIT(11) | len,
>    449								skb_push(skb2, 2));
>    450
>    451					ep = port->in_ep;
>    452					req = usb_ep_alloc_request(ep, GFP_ATOMIC);
>    453					if (!req)
>    454						goto free_skb;
>    455
>    456					req->buf = kmalloc(skb2->len, GFP_KERNEL);
>    457					if (!req->buf)
>    458						goto free_request;
>    459
>    460					ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
>    461					if (!ctx) {
>    462						kfree(req->buf);
>    463						goto free_request;
>    464					}
>    465					ctx->skb = skb2;
>    466					ctx->ep = ep;
>    467
>    468					skb_copy_bits(skb2, 0, req->buf, skb2->len);
>    469					req->length = skb2->len;
>    470					req->complete = eem_cmd_complete;
>    471					req->zero = 1;
>    472					req->context = ctx;
>    473					if (usb_ep_queue(port->in_ep, req, GFP_ATOMIC))
>    474						DBG(cdev, "echo response queue fail\n");
>    475					break;
>    476
>    477				case 1:  /* echo response */
>    478				case 2:  /* suspend hint */
>    479				case 3:  /* response hint */
>    480				case 4:  /* response complete hint */
>    481				case 5:  /* tickle */
>    482				default: /* reserved */
>    483					continue;
>    484				}
>    485			} else {
>    486				u32		crc, crc2;
>    487				struct sk_buff	*skb3;
>    488
>    489				/* check for zero-length EEM packet */
>    490				if (header == 0)
>    491					continue;
>    492
>    493				/* EEM data packet format:
>    494				 * b0..13:	length of ethernet frame
>    495				 * b14:		bmCRC (0 == sentinel, 1 == calculated)
>    496				 * b15:		bmType (0 == data)
>    497				 */
>    498				len = header & 0x3FFF;
>    499				if ((skb->len < len)
>    500						|| (len < (ETH_HLEN + ETH_FCS_LEN))) {
>    501					status = -EINVAL;
>    502					goto error;
>    503				}
>    504
>    505				/* validate CRC */
>    506				if (header & BIT(14)) {
>    507					crc = get_unaligned_le32(skb->data + len
>    508								- ETH_FCS_LEN);
>    509					crc2 = ~crc32_le(~0,
>    510							skb->data, len - ETH_FCS_LEN);
>    511				} else {
>    512					crc = get_unaligned_be32(skb->data + len
>    513								- ETH_FCS_LEN);
>    514					crc2 = 0xdeadbeef;
>    515				}
>    516				if (crc != crc2) {
>    517					DBG(cdev, "invalid EEM CRC\n");
>    518					goto next;
>    519				}
>    520
>    521				skb2 = skb_clone(skb, GFP_ATOMIC);
>    522				if (unlikely(!skb2)) {
>    523					DBG(cdev, "unable to unframe EEM packet\n");
>    524					goto next;
>    525				}
>    526				skb_trim(skb2, len - ETH_FCS_LEN);
>    527
>    528				skb3 = skb_copy_expand(skb2,
>    529							NET_IP_ALIGN,
>    530							0,
>    531							GFP_ATOMIC);
>    532				if (unlikely(!skb3))
>    533					goto free_skb;
>    534
>    535				dev_kfree_skb_any(skb2);
>    536				skb_queue_tail(list, skb3);
>    537			}
>    538	next:
>    539			skb_pull(skb, len);
>    540		} while (skb->len);
>    541
>    542	error:
>    543		dev_kfree_skb_any(skb);
>    544		return status;
>    545
>    546	free_request:
>  > 547		usb_ep_free_request(ep, req);
>    548	free_skb:
>  > 549		dev_kfree_skb_any(skb2);
>    550		goto next;
>    551	}
>    552

Would any other software design options become acceptable for the reduction of duplicate statements
(according to resource management)?

Regards,
Markus





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux