netdev - Re: [PATCH RESEND 2/2] wlcore: align member-assigns in a structure-copy block

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: <CAB3XZEcm4+=dYC-8_m_7YRdET_wPkWUYHcDuy9mCKdxkCFEnQA@mail.gmail.com>
Date:	Sun, 11 Jan 2015 12:22:49 +0200
From:	Eliad Peller <eliad@...ery.com>
To:	Kalle Valo <kvalo@...eaurora.org>
Cc:	Giel van Schijndel <me@...tis.eu>,
	LKML <linux-kernel@...r.kernel.org>,
	"John W. Linville" <linville@...driver.com>,
	Arik Nemtsov <arik@...ery.com>,
	"open list:TI WILINK WIRELES..." <linux-wireless@...r.kernel.org>,
	"open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>
Subject: Re: [PATCH RESEND 2/2] wlcore: align member-assigns in a
 structure-copy block

On Fri, Jan 9, 2015 at 7:03 PM, Kalle Valo <kvalo@...eaurora.org> wrote:
> Giel van Schijndel <me@...tis.eu> writes:
>
>> This highlights the differences (e.g. the bug fixed in the previous
>> commit).
>>
>> Signed-off-by: Giel van Schijndel <me@...tis.eu>
>> ---
>>  drivers/net/wireless/ti/wlcore/acx.c | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
>> index f28fa3b..93a2fa8 100644
>> --- a/drivers/net/wireless/ti/wlcore/acx.c
>> +++ b/drivers/net/wireless/ti/wlcore/acx.c
>> @@ -1715,17 +1715,17 @@ int wl12xx_acx_config_hangover(struct wl1271 *wl)
>>               goto out;
>>       }
>>
>> -     acx->recover_time = cpu_to_le32(conf->recover_time);
>> -     acx->hangover_period = conf->hangover_period;
>> -     acx->dynamic_mode = conf->dynamic_mode;
>> -     acx->early_termination_mode = conf->early_termination_mode;
>> -     acx->max_period = conf->max_period;
>> -     acx->min_period = conf->min_period;
>> -     acx->increase_delta = conf->increase_delta;
>> -     acx->decrease_delta = conf->decrease_delta;
>> -     acx->quiet_time = conf->quiet_time;
>> -     acx->increase_time = conf->increase_time;
>> -     acx->window_size = conf->window_size;
>> +     acx->recover_time               = cpu_to_le32(conf->recover_time);
>> +     acx->hangover_period            = conf->hangover_period;
>> +     acx->dynamic_mode               = conf->dynamic_mode;
>> +     acx->early_termination_mode     = conf->early_termination_mode;
>> +     acx->max_period                 = conf->max_period;
>> +     acx->min_period                 = conf->min_period;
>> +     acx->increase_delta             = conf->increase_delta;
>> +     acx->decrease_delta             = conf->decrease_delta;
>> +     acx->quiet_time                 = conf->quiet_time;
>> +     acx->increase_time              = conf->increase_time;
>> +     acx->window_size                = conf->window_size;
>
> I would like to get an ACK from one of the wlcore developers if I should
> apply this (or not).
>
I don't have a strong opinion here.
However, it looks pretty much redundant to take a random blob (which
was just fixed by a correct patch) and re-indent it.
The rest of the file doesn't follow this style, so i don't see a good
reason to apply it here.

I agree such indentation have some benefit, but it won't help with the
more common use case (of copy-paste error) of copying the wrong field
(i.e. D->a = S->b instead of D->a = S->a).
For these cases the macros suggested by Arend and Johannes will do the
trick. However i usually dislike such macros, as they tend to break
some IDE features (e.g. auto completion).
Maybe we can come up with some nice spatch to catch these cases.

Just my 2c.

Eliad.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html