Re: [GIT PULL] Staging wireless driver for 3.14-rc1

2014-02-01 Thread Greg KH
On Sat, Feb 01, 2014 at 10:43:11AM -0800, Linus Torvalds wrote:
> On Fri, Jan 31, 2014 at 6:32 AM, Greg KH  wrote:
> >
> > Here's a single staging driver for a wireless chipset that has shown up
> > in the SteamBox hardware.  It is merged separately from the "main"
> > staging pull request to sync up with the wireless api changes that came
> > in from the networking tree.
> 
> It causes an interesting warning for me:
> 
> drivers/staging/rtl8821ae/rtl8821ae/dm.c: In function
> ‘rtl8821ae_dm_clear_txpower_tracking_state’:
> drivers/staging/rtl8821ae/rtl8821ae/dm.c:487:31: warning: iteration 2u
> invokes undefined behavior [-Waggressive-loop-optimizations]
>rtldm->bb_swing_idx_ofdm[p] = rtldm->default_ofdm_index;
>^
> drivers/staging/rtl8821ae/rtl8821ae/dm.c:485:2: note: containing loop
>   for (p = RF90_PATH_A; p < MAX_RF_PATH; ++p) {
>   ^
> 
> and gcc is entirely correct: that loop iterates from 0 to 3, and does this:
> 
> rtldm->bb_swing_idx_ofdm[p] = rtldm->default_ofdm_index;
> 
> but the bb_swing_idx_ofdm[] array only has two members. So the last
> two iterations will overwrite bb_swing_idx_ofdm_current and the first
> entry in bb_swing_idx_ofdm_base[].
> 
> Now, the bug does seem to be benign: bb_swing_idx_ofdm_current isn't
> actually ever *used* as far as I can tell, and the first entry of
> bb_swing_idx_ofdm_base[] will have been written with that same
> "rtldm->default_ofdm_index" value.
> 
> But gcc is absolutely correct, and that driver needs fixing.
> 
> I've pulled it and will let it be because it doesn't seem to be an
> issue in practice, but please fix it. The obvious fix would seem to
> change the size of "2" to be "MAX_RF_PATH", but I'll abstain from
> doing those kinds of changes in the merge when it doesn't seem to
> affect the build or functionality).

Ok, thanks, someone's already sent me that patch to fix this up, I'll
queue it up for the post-rc1 set of staging tree fixes.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [GIT PULL] Staging wireless driver for 3.14-rc1

2014-02-01 Thread Linus Torvalds
On Fri, Jan 31, 2014 at 6:32 AM, Greg KH  wrote:
>
> Here's a single staging driver for a wireless chipset that has shown up
> in the SteamBox hardware.  It is merged separately from the "main"
> staging pull request to sync up with the wireless api changes that came
> in from the networking tree.

It causes an interesting warning for me:

drivers/staging/rtl8821ae/rtl8821ae/dm.c: In function
‘rtl8821ae_dm_clear_txpower_tracking_state’:
drivers/staging/rtl8821ae/rtl8821ae/dm.c:487:31: warning: iteration 2u
invokes undefined behavior [-Waggressive-loop-optimizations]
   rtldm->bb_swing_idx_ofdm[p] = rtldm->default_ofdm_index;
   ^
drivers/staging/rtl8821ae/rtl8821ae/dm.c:485:2: note: containing loop
  for (p = RF90_PATH_A; p < MAX_RF_PATH; ++p) {
  ^

and gcc is entirely correct: that loop iterates from 0 to 3, and does this:

rtldm->bb_swing_idx_ofdm[p] = rtldm->default_ofdm_index;

but the bb_swing_idx_ofdm[] array only has two members. So the last
two iterations will overwrite bb_swing_idx_ofdm_current and the first
entry in bb_swing_idx_ofdm_base[].

Now, the bug does seem to be benign: bb_swing_idx_ofdm_current isn't
actually ever *used* as far as I can tell, and the first entry of
bb_swing_idx_ofdm_base[] will have been written with that same
"rtldm->default_ofdm_index" value.

But gcc is absolutely correct, and that driver needs fixing.

I've pulled it and will let it be because it doesn't seem to be an
issue in practice, but please fix it. The obvious fix would seem to
change the size of "2" to be "MAX_RF_PATH", but I'll abstain from
doing those kinds of changes in the merge when it doesn't seem to
affect the build or functionality).

   Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [GIT PULL] Staging wireless driver for 3.14-rc1

2014-02-01 Thread Linus Torvalds
On Fri, Jan 31, 2014 at 6:32 AM, Greg KH gre...@linuxfoundation.org wrote:

 Here's a single staging driver for a wireless chipset that has shown up
 in the SteamBox hardware.  It is merged separately from the main
 staging pull request to sync up with the wireless api changes that came
 in from the networking tree.

It causes an interesting warning for me:

drivers/staging/rtl8821ae/rtl8821ae/dm.c: In function
‘rtl8821ae_dm_clear_txpower_tracking_state’:
drivers/staging/rtl8821ae/rtl8821ae/dm.c:487:31: warning: iteration 2u
invokes undefined behavior [-Waggressive-loop-optimizations]
   rtldm-bb_swing_idx_ofdm[p] = rtldm-default_ofdm_index;
   ^
drivers/staging/rtl8821ae/rtl8821ae/dm.c:485:2: note: containing loop
  for (p = RF90_PATH_A; p  MAX_RF_PATH; ++p) {
  ^

and gcc is entirely correct: that loop iterates from 0 to 3, and does this:

rtldm-bb_swing_idx_ofdm[p] = rtldm-default_ofdm_index;

but the bb_swing_idx_ofdm[] array only has two members. So the last
two iterations will overwrite bb_swing_idx_ofdm_current and the first
entry in bb_swing_idx_ofdm_base[].

Now, the bug does seem to be benign: bb_swing_idx_ofdm_current isn't
actually ever *used* as far as I can tell, and the first entry of
bb_swing_idx_ofdm_base[] will have been written with that same
rtldm-default_ofdm_index value.

But gcc is absolutely correct, and that driver needs fixing.

I've pulled it and will let it be because it doesn't seem to be an
issue in practice, but please fix it. The obvious fix would seem to
change the size of 2 to be MAX_RF_PATH, but I'll abstain from
doing those kinds of changes in the merge when it doesn't seem to
affect the build or functionality).

   Linus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [GIT PULL] Staging wireless driver for 3.14-rc1

2014-02-01 Thread Greg KH
On Sat, Feb 01, 2014 at 10:43:11AM -0800, Linus Torvalds wrote:
 On Fri, Jan 31, 2014 at 6:32 AM, Greg KH gre...@linuxfoundation.org wrote:
 
  Here's a single staging driver for a wireless chipset that has shown up
  in the SteamBox hardware.  It is merged separately from the main
  staging pull request to sync up with the wireless api changes that came
  in from the networking tree.
 
 It causes an interesting warning for me:
 
 drivers/staging/rtl8821ae/rtl8821ae/dm.c: In function
 ‘rtl8821ae_dm_clear_txpower_tracking_state’:
 drivers/staging/rtl8821ae/rtl8821ae/dm.c:487:31: warning: iteration 2u
 invokes undefined behavior [-Waggressive-loop-optimizations]
rtldm-bb_swing_idx_ofdm[p] = rtldm-default_ofdm_index;
^
 drivers/staging/rtl8821ae/rtl8821ae/dm.c:485:2: note: containing loop
   for (p = RF90_PATH_A; p  MAX_RF_PATH; ++p) {
   ^
 
 and gcc is entirely correct: that loop iterates from 0 to 3, and does this:
 
 rtldm-bb_swing_idx_ofdm[p] = rtldm-default_ofdm_index;
 
 but the bb_swing_idx_ofdm[] array only has two members. So the last
 two iterations will overwrite bb_swing_idx_ofdm_current and the first
 entry in bb_swing_idx_ofdm_base[].
 
 Now, the bug does seem to be benign: bb_swing_idx_ofdm_current isn't
 actually ever *used* as far as I can tell, and the first entry of
 bb_swing_idx_ofdm_base[] will have been written with that same
 rtldm-default_ofdm_index value.
 
 But gcc is absolutely correct, and that driver needs fixing.
 
 I've pulled it and will let it be because it doesn't seem to be an
 issue in practice, but please fix it. The obvious fix would seem to
 change the size of 2 to be MAX_RF_PATH, but I'll abstain from
 doing those kinds of changes in the merge when it doesn't seem to
 affect the build or functionality).

Ok, thanks, someone's already sent me that patch to fix this up, I'll
queue it up for the post-rc1 set of staging tree fixes.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/