lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.44L0.1601041600150.1324-100000@iolanthe.rowland.org>
Date:	Mon, 4 Jan 2016 16:01:38 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	"Geyslan G. Bem" <geyslan@...il.com>
cc:	linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	<linux-usb@...r.kernel.org>
Subject: Re: [PATCH 17/17] usb: host: ehci-dbg: refactor fill_periodic_buffer
 function

On Mon, 4 Jan 2016, Geyslan G. Bem wrote:

> This patch fixes a coding style issue reported by checkpatch related to
> many leading tabs, removing a 'do while' loop and making use of goto tag instead.

This is highly questionable.  It's a big amount of code churn, nearly 
impossible to verify visually, just to remove one level of indentation.  
It also introduces an unnecessary backwards "goto", which seems like a 
bad idea.

Alan Stern

> Others changes in this patch are:
>  - Some multiline statements are reduced (718, 729, 780, 786, 790).
>  - A constant is moved to right on line 770.
> 
> Signed-off-by: Geyslan G. Bem <geyslan@...il.com>
> ---
> 
> Notes:
>     Tested by compilation only.
> 
>  drivers/usb/host/ehci-dbg.c | 180 ++++++++++++++++++++++----------------------
>  1 file changed, 88 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> index 2268756..278333d 100644
> --- a/drivers/usb/host/ehci-dbg.c
> +++ b/drivers/usb/host/ehci-dbg.c
> @@ -698,6 +698,8 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>  	 */
>  	spin_lock_irqsave(&ehci->lock, flags);
>  	for (i = 0; i < ehci->periodic_size; i++) {
> +		struct ehci_qh_hw *hw;
> +
>  		p = ehci->pshadow[i];
>  		if (likely(!p.ptr))
>  			continue;
> @@ -707,104 +709,98 @@ static ssize_t fill_periodic_buffer(struct debug_buffer *buf)
>  		size -= temp;
>  		next += temp;
>  
> -		do {
> -			struct ehci_qh_hw *hw;
> -
> -			switch (hc32_to_cpu(ehci, tag)) {
> -			case Q_TYPE_QH:
> -				hw = p.qh->hw;
> -				temp = scnprintf(next, size, " qh%d-%04x/%p",
> -						p.qh->ps.period,
> -						hc32_to_cpup(ehci,
> -							&hw->hw_info2)
> -							/* uframe masks */
> -							& (QH_CMASK | QH_SMASK),
> -						p.qh);
> -				size -= temp;
> -				next += temp;
> -				/* don't repeat what follows this qh */
> -				for (temp = 0; temp < seen_count; temp++) {
> -					if (seen[temp].ptr != p.ptr)
> +do_loop:
> +		switch (hc32_to_cpu(ehci, tag)) {
> +		case Q_TYPE_QH:
> +			hw = p.qh->hw;
> +			temp = scnprintf(next, size, " qh%d-%04x/%p",
> +					p.qh->ps.period,
> +					hc32_to_cpup(ehci, &hw->hw_info2)
> +						/* uframe masks */
> +						& (QH_CMASK | QH_SMASK),
> +					p.qh);
> +			size -= temp;
> +			next += temp;
> +			/* don't repeat what follows this qh */
> +			for (temp = 0; temp < seen_count; temp++) {
> +				if (seen[temp].ptr != p.ptr)
> +					continue;
> +				if (p.qh->qh_next.ptr) {
> +					temp = scnprintf(next, size, " ...");
> +					size -= temp;
> +					next += temp;
> +				}
> +				break;
> +			}
> +			/* show more info the first time around */
> +			if (temp == seen_count) {
> +				u32	scratch = hc32_to_cpup(ehci,
> +						&hw->hw_info1);
> +				struct ehci_qtd	*qtd;
> +				char		*type = "";
> +
> +				/* count tds, get ep direction */
> +				temp = 0;
> +				list_for_each_entry(qtd,
> +						&p.qh->qtd_list,
> +						qtd_list) {
> +					temp++;
> +					switch ((hc32_to_cpu(ehci,
> +						qtd->hw_token) >> 8)
> +							& 0x03) {
> +					case 0:
> +						type = "out";
> +						continue;
> +					case 1:
> +						type = "in";
>  						continue;
> -					if (p.qh->qh_next.ptr) {
> -						temp = scnprintf(next, size,
> -							" ...");
> -						size -= temp;
> -						next += temp;
>  					}
> -					break;
>  				}
> -				/* show more info the first time around */
> -				if (temp == seen_count) {
> -					u32	scratch = hc32_to_cpup(ehci,
> -							&hw->hw_info1);
> -					struct ehci_qtd	*qtd;
> -					char		*type = "";
> -
> -					/* count tds, get ep direction */
> -					temp = 0;
> -					list_for_each_entry(qtd,
> -							&p.qh->qtd_list,
> -							qtd_list) {
> -						temp++;
> -						switch ((hc32_to_cpu(ehci,
> -							qtd->hw_token) >> 8)
> -								& 0x03) {
> -						case 0:
> -							type = "out";
> -							continue;
> -						case 1:
> -							type = "in";
> -							continue;
> -						}
> -					}
>  
> -					temp = scnprintf(next, size,
> -						" (%c%d ep%d%s "
> -						"[%d/%d] q%d p%d)",
> -						speed_char (scratch),
> -						scratch & 0x007f,
> -						(scratch >> 8) & 0x000f, type,
> -						p.qh->ps.usecs,
> -						p.qh->ps.c_usecs,
> -						temp,
> -						0x7ff & (scratch >> 16));
> -
> -					if (seen_count < DBG_SCHED_LIMIT)
> -						seen[seen_count++].qh = p.qh;
> -				} else {
> -					temp = 0;
> -				}
> -				tag = Q_NEXT_TYPE(ehci, hw->hw_next);
> -				p = p.qh->qh_next;
> -				break;
> -			case Q_TYPE_FSTN:
> -				temp = scnprintf(next, size,
> -					" fstn-%8x/%p", p.fstn->hw_prev,
> -					p.fstn);
> -				tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
> -				p = p.fstn->fstn_next;
> -				break;
> -			case Q_TYPE_ITD:
> -				temp = scnprintf(next, size,
> -					" itd/%p", p.itd);
> -				tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
> -				p = p.itd->itd_next;
> -				break;
> -			case Q_TYPE_SITD:
>  				temp = scnprintf(next, size,
> -					" sitd%d-%04x/%p",
> -					p.sitd->stream->ps.period,
> -					hc32_to_cpup(ehci, &p.sitd->hw_uframe)
> -						& 0x0000ffff,
> -					p.sitd);
> -				tag = Q_NEXT_TYPE(ehci, p.sitd->hw_next);
> -				p = p.sitd->sitd_next;
> -				break;
> +					" (%c%d ep%d%s "
> +					"[%d/%d] q%d p%d)",
> +					speed_char (scratch),
> +					scratch & 0x007f,
> +					(scratch >> 8) & 0x000f, type,
> +					p.qh->ps.usecs,
> +					p.qh->ps.c_usecs,
> +					temp,
> +					(scratch >> 16) & 0x7ff);
> +
> +				if (seen_count < DBG_SCHED_LIMIT)
> +					seen[seen_count++].qh = p.qh;
> +			} else {
> +				temp = 0;
>  			}
> -			size -= temp;
> -			next += temp;
> -		} while (p.ptr);
> +			tag = Q_NEXT_TYPE(ehci, hw->hw_next);
> +			p = p.qh->qh_next;
> +			break;
> +		case Q_TYPE_FSTN:
> +			temp = scnprintf(next, size, " fstn-%8x/%p",
> +				p.fstn->hw_prev, p.fstn);
> +			tag = Q_NEXT_TYPE(ehci, p.fstn->hw_next);
> +			p = p.fstn->fstn_next;
> +			break;
> +		case Q_TYPE_ITD:
> +			temp = scnprintf(next, size, " itd/%p", p.itd);
> +			tag = Q_NEXT_TYPE(ehci, p.itd->hw_next);
> +			p = p.itd->itd_next;
> +			break;
> +		case Q_TYPE_SITD:
> +			temp = scnprintf(next, size, " sitd%d-%04x/%p",
> +				p.sitd->stream->ps.period,
> +				hc32_to_cpup(ehci, &p.sitd->hw_uframe)
> +					& 0x0000ffff,
> +				p.sitd);
> +			tag = Q_NEXT_TYPE(ehci, p.sitd->hw_next);
> +			p = p.sitd->sitd_next;
> +			break;
> +		}
> +		size -= temp;
> +		next += temp;
> +		if (p.ptr)
> +			goto do_loop;
>  
>  		temp = scnprintf(next, size, "\n");
>  		size -= temp;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ