| 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: Sat, 12 Dec 2015 13:52:54 -0300
From: "Geyslan G. Bem" <geyslan@...il.com>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Peter Senna Tschudin <peter.senna@...il.com>,
linux-usb@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 01/10] usb: host: ehci-sched: refactor scan_isoc function
2015-12-12 13:15 GMT-03:00 Alan Stern <stern@...land.harvard.edu>:
> On Fri, 11 Dec 2015, Geyslan G. Bem wrote:
>
>> This patch removes an infinite 'for' loop and makes use of the already
>> existing 'restart' tag instead, reducing one leading tab.
>>
>> The comments and code were corrected conforming coding style.
>>
>> Tested by compilation only.
>> Caught by checkpatch:
>> WARNING: Too many leading tabs - consider code refactoring
>>
>> Signed-off-by: Geyslan G. Bem <geyslan@...il.com>
>> ---
>> drivers/usb/host/ehci-sched.c | 203 ++++++++++++++++++++++--------------------
>> 1 file changed, 104 insertions(+), 99 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
>> index f9a3327..86b2484 100644
>> --- a/drivers/usb/host/ehci-sched.c
>> +++ b/drivers/usb/host/ehci-sched.c
>> @@ -2383,6 +2383,9 @@ static void scan_isoc(struct ehci_hcd *ehci)
>> unsigned fmask = ehci->periodic_size - 1;
>> bool modified, live;
>>
>> + union ehci_shadow q, *q_p;
>> + __hc32 type, *hw_p;
>
> These new declarations should line up with the ones above. And there
> shouldn't be a blank line between.
Ok, I'll fix it.
>
>> + while (q.ptr != NULL) {
>> + switch (hc32_to_cpu(ehci, type)) {
>> + case Q_TYPE_ITD:
>> + /*
>> + * If this ITD is still active, leave it for
>> + * later processing ... check the next entry.
>> + * No need to check for activity unless the
>> + * frame is current.
>> + */
>> + if (frame == now_frame && live) {
>> + rmb();
>> + for (uf = 0; uf < 8; uf++) {
>> + if (q.itd->hw_transaction[uf] &
>> + ITD_ACTIVE(ehci))
>
> The style used in this source file does not align continuation lines
> with open parens on the line above. Instead, the continuation lines
> are indented by 2 extra tabs (or maybe 1 extra if 2 would pushd them
> beyond 80 characters).
My Emacs is configured for linux indentation. I'll fix it manually. Or
have you a ready lisp for this style? :-)
Is there a doc like the Documentation/CodingStyle with this different rules?
>
>> + if (uf < 8) {
>> + q_p = &q.itd->itd_next;
>> + hw_p = &q.itd->hw_next;
>> type = Q_NEXT_TYPE(ehci,
>> - q.sitd->hw_next);
>> + q.itd->hw_next);
>
> Ditto.
>
>> + /*
>> + * Take finished ITDs out of the schedule
>> + * and process them: recycle, maybe report
>> + * URB completion. HC won't cache the
>> + * pointer for much longer, if at all.
>> + */
>> + *q_p = q.itd->itd_next;
>> + if (!ehci->use_dummy_qh ||
>> + q.itd->hw_next != EHCI_LIST_END(ehci))
>
> Ditto.
>
>> + *hw_p = q.itd->hw_next;
>> + else
>> + *hw_p = cpu_to_hc32(ehci,
>> + ehci->dummy->qh_dma);
>
> Ditto.
>
>> + case Q_TYPE_SITD:
>> + /*
>> + * If this SITD is still active, leave it for
>> + * later processing ... check the next entry.
>> + * No need to check for activity unless the
>> + * frame is current.
>> + */
>> + if (((frame == now_frame) ||
>> + (((frame + 1) & fmask) == now_frame))
>> + && live
>> + && (q.sitd->hw_results &
>> + SITD_ACTIVE(ehci))) {
>
> Although these lines didn't follow the style used in the rest of the
> file, they should do so now.
Ok.
>
>> +
>> + q_p = &q.sitd->sitd_next;
>> + hw_p = &q.sitd->hw_next;
>> + type = Q_NEXT_TYPE(ehci,
>> + q.sitd->hw_next);
>
> Ditto
>
>> + * Take finished SITDs out of the schedule
>> + * and process them: recycle, maybe report
>> + * URB completion.
>> + */
>> + *q_p = q.sitd->sitd_next;
>> + if (!ehci->use_dummy_qh ||
>> + q.sitd->hw_next != EHCI_LIST_END(ehci))
>
> Ditto.
>
>> + *hw_p = q.sitd->hw_next;
>> + else
>> + *hw_p = cpu_to_hc32(ehci,
>> + ehci->dummy->qh_dma);
>
> Ditto.
>
>> + type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
>> + wmb();
>> + modified = sitd_complete(ehci, q.sitd);
>> + q = *q_p;
>> break;
>> + default:
>> + ehci_dbg(ehci, "corrupt type %d frame %d shadow %p\n",
>> + type, frame, q.ptr);
>
> Ditto.
>
> Alan Stern
>
--
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