* [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest
2015-01-04 18:00 [PATCH 1/2] Align member-assigns in a structure-copy block Giel van Schijndel
@ 2015-01-04 18:00 ` Giel van Schijndel
2015-01-04 23:02 ` Giel van Schijndel
2015-01-05 9:54 ` Johannes Berg
2015-01-04 23:02 ` [PATCH 1/2] Align member-assigns in a structure-copy block Giel van Schijndel
` (3 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Giel van Schijndel @ 2015-01-04 18:00 UTC (permalink / raw)
To: linux-kernel
Cc: Giel van Schijndel, Kalle Valo, Eliad Peller, John W. Linville,
Arik Nemtsov, open list:TI WILINK WIRELES...,
open list:NETWORKING DRIVERS
---
drivers/net/wireless/ti/wlcore/acx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
index beb354c..93a2fa8 100644
--- a/drivers/net/wireless/ti/wlcore/acx.c
+++ b/drivers/net/wireless/ti/wlcore/acx.c
@@ -1725,7 +1725,7 @@ int wl12xx_acx_config_hangover(struct wl1271 *wl)
acx->decrease_delta = conf->decrease_delta;
acx->quiet_time = conf->quiet_time;
acx->increase_time = conf->increase_time;
- acx->window_size = acx->window_size;
+ acx->window_size = conf->window_size;
ret = wl1271_cmd_configure(wl, ACX_CONFIG_HANGOVER, acx,
sizeof(*acx));
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest
2015-01-04 18:00 ` [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest Giel van Schijndel
@ 2015-01-04 23:02 ` Giel van Schijndel
2015-01-05 9:54 ` Johannes Berg
1 sibling, 0 replies; 18+ messages in thread
From: Giel van Schijndel @ 2015-01-04 23:02 UTC (permalink / raw)
To: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 210 bytes --]
On Sun, Jan 04, 2015 at 19:00:23 +0100, Giel van Schijndel wrote:
> ---
Forgot to:
Signed-off-by: Giel van Schijndel <me@mortis•eu>
--
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest
2015-01-04 18:00 ` [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest Giel van Schijndel
2015-01-04 23:02 ` Giel van Schijndel
@ 2015-01-05 9:54 ` Johannes Berg
2015-01-07 19:18 ` Giel van Schijndel
1 sibling, 1 reply; 18+ messages in thread
From: Johannes Berg @ 2015-01-05 9:54 UTC (permalink / raw)
To: Giel van Schijndel
Cc: linux-kernel, Kalle Valo, Eliad Peller, John W. Linville,
Arik Nemtsov, open list:TI WILINK WIRELES...,
open list:NETWORKING DRIVERS
On Sun, 2015-01-04 at 19:00 +0100, Giel van Schijndel wrote:
> ---
> drivers/net/wireless/ti/wlcore/acx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
> index beb354c..93a2fa8 100644
> --- a/drivers/net/wireless/ti/wlcore/acx.c
> +++ b/drivers/net/wireless/ti/wlcore/acx.c
> @@ -1725,7 +1725,7 @@ int wl12xx_acx_config_hangover(struct wl1271 *wl)
> acx->decrease_delta = conf->decrease_delta;
> acx->quiet_time = conf->quiet_time;
> acx->increase_time = conf->increase_time;
> - acx->window_size = acx->window_size;
> + acx->window_size = conf->window_size;
It would be far better to fix the bug *first*, that way the bugfix can
be cherry-picked/applied to trees that don't have the alignment.
(And anyway I question the value of the alignment - if you really want
to make this bug disappear you could perhaps use a macro)
johannes
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest
2015-01-05 9:54 ` Johannes Berg
@ 2015-01-07 19:18 ` Giel van Schijndel
2015-01-07 22:16 ` Johannes Berg
0 siblings, 1 reply; 18+ messages in thread
From: Giel van Schijndel @ 2015-01-07 19:18 UTC (permalink / raw)
To: Johannes Berg, Andy Shevchenko
Cc: linux-kernel, Kalle Valo, Eliad Peller, John W. Linville,
Arik Nemtsov, open list:TI WILINK WIRELES...,
open list:NETWORKING DRIVERS
[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]
On Mon, Jan 05, 2015 at 10:54:31 +0100, Johannes Berg wrote:
> On Sun, 2015-01-04 at 19:00 +0100, Giel van Schijndel wrote:
>> ---
>> drivers/net/wireless/ti/wlcore/acx.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
>> index beb354c..93a2fa8 100644
>> --- a/drivers/net/wireless/ti/wlcore/acx.c
>> +++ b/drivers/net/wireless/ti/wlcore/acx.c
>> @@ -1725,7 +1725,7 @@ int wl12xx_acx_config_hangover(struct wl1271 *wl)
>> acx->decrease_delta = conf->decrease_delta;
>> acx->quiet_time = conf->quiet_time;
>> acx->increase_time = conf->increase_time;
>> - acx->window_size = acx->window_size;
>> + acx->window_size = conf->window_size;
>
> It would be far better to fix the bug *first*, that way the bugfix can
> be cherry-picked/applied to trees that don't have the alignment.
I agree on the ordering.
As for:
> (And anyway I question the value of the alignment - if you really want
> to make this bug disappear you could perhaps use a macro)
And:
On Wed, Jan 07, 2015 at 20:40:57 +0200, Andy Shevchenko wrote:
> On Sun, Jan 4, 2015 at 8:00 PM, Giel van Schijndel <me@mortis•eu> wrote:
> > This highlights the differences (errors).
>
> Seems like patch for the patch. Just fix an error like it's done here:
> http://www.spinics.net/lists/linux-wireless/msg131667.html
IMO the aligned block of code has the significant advantage of taking
advantage of humans' ability to spot things that break a pattern. Which
in this case becomes *very* visible when properly aligned, because
without the alignment there is no (visual) pattern (or at least not one
very suitable for my "visual processing system", I know the same applies
to at least some others).
--
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest
2015-01-07 19:18 ` Giel van Schijndel
@ 2015-01-07 22:16 ` Johannes Berg
2015-01-07 23:06 ` Arend van Spriel
0 siblings, 1 reply; 18+ messages in thread
From: Johannes Berg @ 2015-01-07 22:16 UTC (permalink / raw)
To: Giel van Schijndel
Cc: Andy Shevchenko, linux-kernel, Kalle Valo, Eliad Peller,
John W. Linville, Arik Nemtsov, open list:TI WILINK WIRELES...,
open list:NETWORKING DRIVERS
On Wed, 2015-01-07 at 20:18 +0100, Giel van Schijndel wrote:
> IMO the aligned block of code has the significant advantage of taking
> advantage of humans' ability to spot things that break a pattern. Which
> in this case becomes *very* visible when properly aligned, because
> without the alignment there is no (visual) pattern (or at least not one
> very suitable for my "visual processing system", I know the same applies
> to at least some others).
Yeah, well, but why even invoke that "visual processing system"?
If you look, for example, at the __skb_clone function it just uses a
macro:
#define C(x) n->x = skb->x
and then
C(len);
C(data_len);
etc.
johannes
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest
2015-01-07 22:16 ` Johannes Berg
@ 2015-01-07 23:06 ` Arend van Spriel
0 siblings, 0 replies; 18+ messages in thread
From: Arend van Spriel @ 2015-01-07 23:06 UTC (permalink / raw)
To: Johannes Berg
Cc: Giel van Schijndel, Andy Shevchenko, linux-kernel, Kalle Valo,
Eliad Peller, John W. Linville, Arik Nemtsov,
open list:TI WILINK WIRELES..., NETWORKING DRIVERS
On 01/07/15 23:16, Johannes Berg wrote:
> On Wed, 2015-01-07 at 20:18 +0100, Giel van Schijndel wrote:
>
>> IMO the aligned block of code has the significant advantage of taking
>> advantage of humans' ability to spot things that break a pattern. Which
>> in this case becomes *very* visible when properly aligned, because
>> without the alignment there is no (visual) pattern (or at least not one
>> very suitable for my "visual processing system", I know the same applies
>> to at least some others).
>
> Yeah, well, but why even invoke that "visual processing system"?
>
> If you look, for example, at the __skb_clone function it just uses a
> macro:
>
> #define C(x) n->x = skb->x
This requires fixed names so I generally prefer to add them:
#define C(d, s, f) (d)->f = (s)->f
> and then
>
> C(len);
> C(data_len);
C(acx, conf, window_size);
C(acx, conf, increase_time);
Regards,
Arend
>
> etc.
>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger•kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] Align member-assigns in a structure-copy block
2015-01-04 18:00 [PATCH 1/2] Align member-assigns in a structure-copy block Giel van Schijndel
2015-01-04 18:00 ` [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest Giel van Schijndel
@ 2015-01-04 23:02 ` Giel van Schijndel
2015-01-05 9:17 ` Kalle Valo
2015-01-05 9:16 ` Kalle Valo
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Giel van Schijndel @ 2015-01-04 23:02 UTC (permalink / raw)
To: linux-kernel
Cc: Kalle Valo, Eliad Peller, John W. Linville, Arik Nemtsov,
open list:TI WILINK WIRELES..., open list:NETWORKING DRIVERS
[-- Attachment #1: Type: text/plain, Size: 377 bytes --]
On Sun, Jan 04, 2015 at 19:00:22 +0100, Giel van Schijndel wrote:
> This highlights the differences (errors).
> ---
Forgot to:
Signed-off-by: Giel van Schijndel <me@mortis•eu>
--
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
--
"Walking on water and developing software from a specification are easy
if both are frozen."
-- Edward V Berard
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 1/2] Align member-assigns in a structure-copy block
2015-01-04 23:02 ` [PATCH 1/2] Align member-assigns in a structure-copy block Giel van Schijndel
@ 2015-01-05 9:17 ` Kalle Valo
0 siblings, 0 replies; 18+ messages in thread
From: Kalle Valo @ 2015-01-05 9:17 UTC (permalink / raw)
To: Giel van Schijndel
Cc: linux-kernel, Eliad Peller, John W. Linville, Arik Nemtsov,
linux-wireless, netdev
Giel van Schijndel <me@mortis•eu> writes:
> On Sun, Jan 04, 2015 at 19:00:22 +0100, Giel van Schijndel wrote:
>> This highlights the differences (errors).
>> ---
>
> Forgot to:
> Signed-off-by: Giel van Schijndel <me@mortis•eu>
I don't normally edit patches (don't have time for that), so please
resend.
--
Kalle Valo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] Align member-assigns in a structure-copy block
2015-01-04 18:00 [PATCH 1/2] Align member-assigns in a structure-copy block Giel van Schijndel
2015-01-04 18:00 ` [PATCH 2/2] Fix copy-paste bug: assign from src struct not dest Giel van Schijndel
2015-01-04 23:02 ` [PATCH 1/2] Align member-assigns in a structure-copy block Giel van Schijndel
@ 2015-01-05 9:16 ` Kalle Valo
[not found] ` <1420394427-19509-1-git-send-email-me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org>
2015-01-07 19:38 ` [PATCH RESEND 1/2] wlcore: fix copy-paste bug: assign from src struct not dest Giel van Schijndel
4 siblings, 0 replies; 18+ messages in thread
From: Kalle Valo @ 2015-01-05 9:16 UTC (permalink / raw)
To: Giel van Schijndel
Cc: linux-kernel, Eliad Peller, John W. Linville, Arik Nemtsov,
linux-wireless, netdev
Giel van Schijndel <me@mortis•eu> writes:
> This highlights the differences (errors).
> ---
> drivers/net/wireless/ti/wlcore/acx.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
Please prefix the patch titles with "wlcore: ".
--
Kalle Valo
^ permalink raw reply [flat|nested] 18+ messages in thread[parent not found: <1420394427-19509-1-git-send-email-me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org>]
* Re: [PATCH 1/2] Align member-assigns in a structure-copy block
[not found] ` <1420394427-19509-1-git-send-email-me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org>
@ 2015-01-07 18:40 ` Andy Shevchenko
0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2015-01-07 18:40 UTC (permalink / raw)
To: Giel van Schijndel
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public•gmane.org, Kalle Valo,
Eliad Peller, John W. Linville, Arik Nemtsov,
open list:TI WILINK WIRELES..., open list:NETWORKING DRIVERS
On Sun, Jan 4, 2015 at 8:00 PM, Giel van Schijndel <me-sZ9Uef1cvPWHXe+LvDLADg@public•gmane.org> wrote:
> This highlights the differences (errors).
Seems like patch for the patch. Just fix an error like it's done here:
http://www.spinics.net/lists/linux-wireless/msg131667.html
> ---
> 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 b924cea..beb354c 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 = acx->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 = acx->window_size;
>
> ret = wl1271_cmd_configure(wl, ACX_CONFIG_HANGOVER, acx,
> sizeof(*acx));
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RESEND 1/2] wlcore: fix copy-paste bug: assign from src struct not dest
2015-01-04 18:00 [PATCH 1/2] Align member-assigns in a structure-copy block Giel van Schijndel
` (3 preceding siblings ...)
[not found] ` <1420394427-19509-1-git-send-email-me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org>
@ 2015-01-07 19:38 ` Giel van Schijndel
2015-01-07 19:38 ` [PATCH RESEND 2/2] wlcore: align member-assigns in a structure-copy block Giel van Schijndel
2015-01-23 17:07 ` [RESEND, 1/2] wlcore: fix copy-paste bug: assign from src struct not dest Kalle Valo
4 siblings, 2 replies; 18+ messages in thread
From: Giel van Schijndel @ 2015-01-07 19:38 UTC (permalink / raw)
To: linux-kernel
Cc: Giel van Schijndel, Kalle Valo, John W. Linville, Eliad Peller,
Arik Nemtsov, open list:TI WILINK WIRELES...,
open list:NETWORKING DRIVERS
Signed-off-by: Giel van Schijndel <me@mortis•eu>
Reported-at: http://www.viva64.com/en/b/0299/
---
drivers/net/wireless/ti/wlcore/acx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
index b924cea..f28fa3b 100644
--- a/drivers/net/wireless/ti/wlcore/acx.c
+++ b/drivers/net/wireless/ti/wlcore/acx.c
@@ -1725,7 +1725,7 @@ int wl12xx_acx_config_hangover(struct wl1271 *wl)
acx->decrease_delta = conf->decrease_delta;
acx->quiet_time = conf->quiet_time;
acx->increase_time = conf->increase_time;
- acx->window_size = acx->window_size;
+ acx->window_size = conf->window_size;
ret = wl1271_cmd_configure(wl, ACX_CONFIG_HANGOVER, acx,
sizeof(*acx));
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH RESEND 2/2] wlcore: align member-assigns in a structure-copy block
2015-01-07 19:38 ` [PATCH RESEND 1/2] wlcore: fix copy-paste bug: assign from src struct not dest Giel van Schijndel
@ 2015-01-07 19:38 ` Giel van Schijndel
[not found] ` <1420659525-22975-2-git-send-email-me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org>
2015-01-23 17:07 ` [RESEND, 1/2] wlcore: fix copy-paste bug: assign from src struct not dest Kalle Valo
1 sibling, 1 reply; 18+ messages in thread
From: Giel van Schijndel @ 2015-01-07 19:38 UTC (permalink / raw)
To: linux-kernel
Cc: Giel van Schijndel, Kalle Valo, Eliad Peller, John W. Linville,
Arik Nemtsov, open list:TI WILINK WIRELES...,
open list:NETWORKING DRIVERS
This highlights the differences (e.g. the bug fixed in the previous
commit).
Signed-off-by: Giel van Schijndel <me@mortis•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;
ret = wl1271_cmd_configure(wl, ACX_CONFIG_HANGOVER, acx,
sizeof(*acx));
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RESEND, 1/2] wlcore: fix copy-paste bug: assign from src struct not dest
2015-01-07 19:38 ` [PATCH RESEND 1/2] wlcore: fix copy-paste bug: assign from src struct not dest Giel van Schijndel
2015-01-07 19:38 ` [PATCH RESEND 2/2] wlcore: align member-assigns in a structure-copy block Giel van Schijndel
@ 2015-01-23 17:07 ` Kalle Valo
1 sibling, 0 replies; 18+ messages in thread
From: Kalle Valo @ 2015-01-23 17:07 UTC (permalink / raw)
To: Giel van Schijndel
Cc: linux-kernel, Giel van Schijndel, John W. Linville, Eliad Peller,
Arik Nemtsov, open list:TI WILINK WIRELES...,
open list:NETWORKING DRIVERS
> Signed-off-by: Giel van Schijndel <me@mortis•eu>
> Reported-at: http://www.viva64.com/en/b/0299/
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
^ permalink raw reply [flat|nested] 18+ messages in thread