| 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
| ||
|
Date: Mon, 4 Jan 2016 22:19:05 -0300
From: "Geyslan G. Bem" <geyslan@...il.com>
To: Alan Stern <stern@...land.harvard.edu>
Cc: LKML <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
2016-01-04 18:01 GMT-03:00 Alan Stern <stern@...land.harvard.edu>:
> 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.
After hear you I agree. I saw that others drivers uses similar
structure (fotg210-hcd.c and ohci-dbg.c), but they have less code. It
would be the case in this file of moving code to a new function? If
not, please disregard this patch.
>
> 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;
>>
>
--
Regards,
Geyslan G. Bem
hackingbits.com
--
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