linux-kernel - Re: [PATCH 1/2] staging: et131x: Fix checkpatch whitespace warnings

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] [day] [month] [year] [list]

Date:	Sat, 10 Sep 2011 17:40:19 +0100
From:	Mark Einon <mark.einon@...il.com>
To:	Joe Perches <joe@...ches.com>
Cc:	gregkh@...e.de, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org, o.hartmann@...ovital.com
Subject: Re: [PATCH 1/2] staging: et131x: Fix checkpatch whitespace warnings

On 10 September 2011 06:41, Joe Perches <joe@...ches.com> wrote:
> On Sat, 2011-09-03 at 12:49 +0100, Mark Einon wrote:
>> Some of my previous hacking attempts have not been following the rules.
>> All fixes either lines > 80 chars or whitespace corrections (spaces->tabs etc).
> []
>> diff --git a/drivers/staging/et131x/et1310_mac.c b/drivers/staging/et131x/et1310_mac.c
>> index ab85cb3..36f2168 100644
>> --- a/drivers/staging/et131x/et1310_mac.c
>> +++ b/drivers/staging/et131x/et1310_mac.c
>> @@ -191,7 +191,8 @@ void et1310_config_mac_regs2(struct et131x_adapter *adapter)
>>       cfg1 |= CFG1_RX_ENABLE | CFG1_TX_ENABLE | CFG1_TX_FLOW;
>>       /* Initialize loop back to off */
>>       cfg1 &= ~(CFG1_LOOPBACK | CFG1_RX_FLOW);
>> -     if (adapter->flowcontrol == FLOW_RXONLY || adapter->flowcontrol == FLOW_BOTH)
>> +     if (adapter->flowcontrol == FLOW_RXONLY ||
>> +                             adapter->flowcontrol == FLOW_BOTH)
>
> Especially in cases like this, it helps readability
> to align to open parenthesis like:
>
>        if (adapter->flowcontrol == FLOW_RXONLY ||
>            adapter->flowcontrol == FLOW_BOTH)
>

Thanks, noted. I'll bear it in mind for next time...

> []
>> @@ -641,7 +641,8 @@ static inline void free_send_packet(struct et131x_adapter *adapter,
>>                * they point to
>>                */
>>               do {
>> -                     desc = (struct tx_desc *)(adapter->tx_ring.tx_desc_ring +
>> +                     desc = (struct tx_desc *)
>> +                                 (adapter->tx_ring.tx_desc_ring +
>>                                               INDEX10(tcb->index_start));
>
> Isn't this an unnecessary cast?
> Perhaps it's better as:
>
>                        u32 i10 = INDEX10(tcb->index_start);
>
>                        desc = &adapter->tx_ring.tx_desc_ring[i10];

That sounds like a good idea. I'll add it to the list when I go
through the tx/rx code for the driver.

Cheers,

Mark
--
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/