[PATCH] b43: Eliminate compilation warning in b43_op_set_key

[PATCH] b43: Eliminate compilation warning in b43_op_set_key

[Larry Finger]

A recent pull from wireless testing generates the following warning:
=20
   CC [M]  drivers/net/wireless/b43/main.o
 drivers/net/wireless/b43/main.c: In function =E2=80=98b43_op_set_key=E2=
=80=99:
 drivers/net/wireless/b43/main.c:3636: warning: pointer type mismatch
 in conditional expression

This fix was suggested by Johannes Berg <johannes@sipsolutions.net>.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

Index: wireless-testing/drivers/net/wireless/b43/main.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- wireless-testing.orig/drivers/net/wireless/b43/main.c
+++ wireless-testing/drivers/net/wireless/b43/main.c
@@ -3630,10 +3630,12 @@ static int b43_op_set_key(struct ieee802
=20
 out_unlock:
 	if (!err) {
+		u8 bcast[ETH_ALEN];
+		memset(bcast, 0xff, ETH_ALEN);
 		b43dbg(wl, "%s hardware based encryption for keyidx: %d, "
 		       "mac: %pM\n",
 		       cmd =3D=3D SET_KEY ? "Using" : "Disabling", key->keyidx,
-		       sta ? sta->addr : "<group key>");
+		       sta ? sta->addr : bcast);
 		b43_dump_keymemory(dev);
 	}
 	write_unlock(&wl->tx_lock);
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] b43: Eliminate compilation warning in b43_op_set_key

[John W. Linville]

On Tue, Jan 13, 2009 at 06:20:25PM -0600, Larry Finger wrote:
> A recent pull from wireless testing generates the following warning:
> =20
>    CC [M]  drivers/net/wireless/b43/main.o
>  drivers/net/wireless/b43/main.c: In function =E2=80=98b43_op_set_key=
=E2=80=99:
>  drivers/net/wireless/b43/main.c:3636: warning: pointer type mismatch
>  in conditional expression
>=20
> This fix was suggested by Johannes Berg <johannes@sipsolutions.net>.
>=20
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>=20
> Index: wireless-testing/drivers/net/wireless/b43/main.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- wireless-testing.orig/drivers/net/wireless/b43/main.c
> +++ wireless-testing/drivers/net/wireless/b43/main.c
> @@ -3630,10 +3630,12 @@ static int b43_op_set_key(struct ieee802
> =20
>  out_unlock:
>  	if (!err) {
> +		u8 bcast[ETH_ALEN];
> +		memset(bcast, 0xff, ETH_ALEN);

Isn't there a statically-allocated array w/ the broadcast MAC in it
somewhere already?

>  		b43dbg(wl, "%s hardware based encryption for keyidx: %d, "
>  		       "mac: %pM\n",
>  		       cmd =3D=3D SET_KEY ? "Using" : "Disabling", key->keyidx,
> -		       sta ? sta->addr : "<group key>");
> +		       sta ? sta->addr : bcast);
>  		b43_dump_keymemory(dev);
>  	}
>  	write_unlock(&wl->tx_lock);
>=20

--=20
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] b43: Eliminate compilation warning in b43_op_set_key

[Larry Finger]

John W. Linville wrote:
> On Tue, Jan 13, 2009 at 06:20:25PM -0600, Larry Finger wrote:
>> A recent pull from wireless testing generates the following warning:
>> =20
>>    CC [M]  drivers/net/wireless/b43/main.o
>>  drivers/net/wireless/b43/main.c: In function =E2=80=98b43_op_set_ke=
y=E2=80=99:
>>  drivers/net/wireless/b43/main.c:3636: warning: pointer type mismatc=
h
>>  in conditional expression
>>
>> This fix was suggested by Johannes Berg <johannes@sipsolutions.net>.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> ---
>>
>> Index: wireless-testing/drivers/net/wireless/b43/main.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- wireless-testing.orig/drivers/net/wireless/b43/main.c
>> +++ wireless-testing/drivers/net/wireless/b43/main.c
>> @@ -3630,10 +3630,12 @@ static int b43_op_set_key(struct ieee802
>> =20
>>  out_unlock:
>>  	if (!err) {
>> +		u8 bcast[ETH_ALEN];
>> +		memset(bcast, 0xff, ETH_ALEN);
>=20
> Isn't there a statically-allocated array w/ the broadcast MAC in it
> somewhere already?
>=20

Not that I could find.

There are several places in mac80211 where the broadcast MAC is
created on the fly. None of them would benefit from having a static
array - one would just substitute memcpy for memset and possibly trash
the cache.

There is an array broadcast[MAX_ADDR_LEN] in struct net_device, but it
would need to be initialized with 0xFF's before usage.

I still think that b43 needs to create this array, and that the best
place to do it is in the error path that is unlikely to be executed.

Larry
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] b43: Eliminate compilation warning in b43_op_set_key

[Michael Buesch]

On Wednesday 14 January 2009 16:47:09 Larry Finger wrote:
> John W. Linville wrote:
> > On Tue, Jan 13, 2009 at 06:20:25PM -0600, Larry Finger wrote:
> >> A recent pull from wireless testing generates the following warnin=
g:
> >> =20
> >>    CC [M]  drivers/net/wireless/b43/main.o
> >>  drivers/net/wireless/b43/main.c: In function =E2=80=98b43_op_set_=
key=E2=80=99:
> >>  drivers/net/wireless/b43/main.c:3636: warning: pointer type misma=
tch
> >>  in conditional expression
> >>
> >> This fix was suggested by Johannes Berg <johannes@sipsolutions.net=
>.
> >>
> >> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> >> ---
> >>
> >> Index: wireless-testing/drivers/net/wireless/b43/main.c
> >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> >> --- wireless-testing.orig/drivers/net/wireless/b43/main.c
> >> +++ wireless-testing/drivers/net/wireless/b43/main.c
> >> @@ -3630,10 +3630,12 @@ static int b43_op_set_key(struct ieee802
> >> =20
> >>  out_unlock:
> >>  	if (!err) {
> >> +		u8 bcast[ETH_ALEN];
> >> +		memset(bcast, 0xff, ETH_ALEN);
> >=20
> > Isn't there a statically-allocated array w/ the broadcast MAC in it
> > somewhere already?
> >=20
>=20
> Not that I could find.
>=20
> There are several places in mac80211 where the broadcast MAC is
> created on the fly. None of them would benefit from having a static
> array - one would just substitute memcpy for memset and possibly tras=
h
> the cache.
>=20
> There is an array broadcast[MAX_ADDR_LEN] in struct net_device, but i=
t
> would need to be initialized with 0xFF's before usage.
>=20
> I still think that b43 needs to create this array, and that the best
> place to do it is in the error path that is unlikely to be executed.

This is not an error path.
Why don't you simply define
	static const u8 bcast_addr[] =3D { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,=
 };
It has zero runtime overhead and is much easier to understand.
And the linker will merge the arrays, if there are multiple of these ar=
rays
in the compilation unit.

Note that you might need to wrap this into
#if B43_DEBUG
#endif
to avoid a compilation warning on non-debug builds in any case

--=20
Greetings, Michael.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html