Re: [PATCH 7/9] usb: dwc2: host: use list_for_each_entry_safe

Re: [PATCH 7/9] usb: dwc2: host: use list_for_each_entry_safe

[Julia Lawall]

The call on line 1120 looks suspicious, because qtd could be non-null but 
also not a valid element, if the loop has exited normally.

julia

On Sat, 19 Dec 2015, kbuild test robot wrote:

> CC: kbuild-all@01.org
> In-Reply-To: <c123b5df7c51a27f33f0d423bf4752a221559fd1.1450455486.git.geliangtang@163.com>
> TO: Geliang Tang <geliangtang@163.com>
> CC: John Youn <johnyoun@synopsys.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Geliang Tang <geliangtang@163.com>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
> 
> Hi Geliang,
> 
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v4.4-rc5 next-20151218]
> 
> url:    https://github.com/0day-ci/linux/commits/Geliang-Tang/usb-host-fotg210-use-list_for_each_entry_safe/20151219-003955
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> :::::: branch date: 2 hours ago
> :::::: commit date: 2 hours ago
> 
> >> drivers/usb/dwc2/hcd_ddma.c:1119:11-14: ERROR: invalid reference to the index variable of the iterator on line 1096
> 
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 61366173f3eb9bee0d5cd22e6a7c5407b14087cb
> vim +1119 drivers/usb/dwc2/hcd_ddma.c
> 
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1090  	if (chan->halt_status == DWC2_HC_XFER_URB_DEQUEUE) {
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1091  		list_for_each_entry(qtd, &qh->qtd_list, qtd_list_entry)
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1092  			qtd->in_process = 0;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1093  		return;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1094  	}
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1095  
> 61366173 drivers/usb/dwc2/hcd_ddma.c     Geliang Tang   2015-12-19 @1096  	list_for_each_entry_safe(qtd, tmp, &qh->qtd_list, qtd_list_entry) {
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1097  		int i;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1098  
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1099  		xfer_done = 0;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1100  
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1101  		for (i = 0; i < qtd->n_desc; i++) {
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1102  			if (dwc2_process_non_isoc_desc(hsotg, chan, chnum, qtd,
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1103  						       desc_num, halt_status,
> fbd1cd20 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-11-22  1104  						       &xfer_done)) {
> fbd1cd20 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-11-22  1105  				qtd = NULL;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1106  				break;
> fbd1cd20 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-11-22  1107  			}
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1108  			desc_num++;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1109  		}
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1110  	}
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1111  
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1112  	if (qh->ep_type != USB_ENDPOINT_XFER_CONTROL) {
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1113  		/*
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1114  		 * Resetting the data toggle for bulk and interrupt endpoints
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1115  		 * in case of stall. See handle_hc_stall_intr().
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1116  		 */
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1117  		if (halt_status == DWC2_HC_XFER_STALL)
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1118  			qh->data_toggle = DWC2_HC_PID_DATA0;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11 @1119  		else if (qtd)
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1120  			dwc2_hcd_save_data_toggle(hsotg, chan, chnum, qtd);
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1121  	}
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1122  
> 
> :::::: The code at line 1119 was first introduced by commit
> :::::: dc4c76e7b22cdcc1ba71ff87edee55f464e01658 staging: HCD descriptor DMA support for the DWC2 driver
> 
> :::::: TO: Paul Zimmerman <Paul.Zimmerman@synopsys.com>
> :::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 

[PATCH 7/9] usb: dwc2: host: use list_for_each_entry_safe

[Geliang Tang]

Use list_for_each_entry_safe() instead of list_for_each_safe() to
simplify the code.

Signed-off-by: Geliang Tang <geliangtang@163.com>
---
 drivers/usb/dwc2/hcd_ddma.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 36606fc..14e004d 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -1222,9 +1222,8 @@ static void dwc2_complete_non_isoc_xfer_ddma(struct dwc2_hsotg *hsotg,
 					     int chnum,
 					     enum dwc2_halt_status halt_status)
 {
-	struct list_head *qtd_item, *qtd_tmp;
 	struct dwc2_qh *qh = chan->qh;
-	struct dwc2_qtd *qtd = NULL;
+	struct dwc2_qtd *qtd = NULL, *tmp;
 	int xfer_done;
 	int desc_num = 0;
 
@@ -1234,10 +1233,9 @@ static void dwc2_complete_non_isoc_xfer_ddma(struct dwc2_hsotg *hsotg,
 		return;
 	}
 
-	list_for_each_safe(qtd_item, qtd_tmp, &qh->qtd_list) {
+	list_for_each_entry_safe(qtd, tmp, &qh->qtd_list, qtd_list_entry) {
 		int i;
 
-		qtd = list_entry(qtd_item, struct dwc2_qtd, qtd_list_entry);
 		xfer_done = 0;
 
 		for (i = 0; i < qtd->n_desc; i++) {
-- 
2.5.0