From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from crystal.sipsolutions.net ([195.210.38.204]:54230 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030342AbXCVKFF (ORCPT ); Thu, 22 Mar 2007 06:05:05 -0400 Subject: Re: [PATCH 1/4] mac80211: Coding style cleanups From: Johannes Berg To: andy@warmcat.com Cc: linux-wireless@vger.kernel.org In-Reply-To: <20070320104104.112865979@warmcat.com> References: <20070320103955.600509703@warmcat.com> <20070320104104.112865979@warmcat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-NUhPxIb5q+tWW2XfJ0P9" Date: Wed, 21 Mar 2007 19:58:23 +0100 Message-Id: <1174503503.3944.50.camel@johannes.berg> Mime-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: --=-NUhPxIb5q+tWW2XfJ0P9 Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Tue, 2007-03-20 at 10:39 +0000, andy@warmcat.com wrote: I don't really see why this is necessary at all, but anyway. > - (tx->fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_DATA)) { > + (tx->fc & IEEE80211_FCTL_FTYPE) > +=3D=3D IEEE80211_FTYPE_DATA)) { That's totally borked. > - if (unlikely((tx->fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_DA= TA && > + if (unlikely((tx->fc & IEEE80211_FCTL_FTYPE) =3D=3D > + IEEE80211_FTYPE_DATA && Indent that a bit more so it's clear it belongs to the line above and isn't a condition itself. =20 > if (unlikely(!sta || > ((tx->fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_MGMT && > - (tx->fc & IEEE80211_FCTL_STYPE) =3D=3D IEEE80211_STYPE_PROBE_RES= P))) > + (tx->fc & IEEE80211_FCTL_STYPE) =3D=3D > + IEEE80211_STYPE_PROBE_RESP))) ditto. > @@ -1170,7 +1178,8 @@ static int __ieee80211_tx(struct ieee80211_local *l= ocal, struct sk_buff *skb, > IEEE80211_TXCTL_RATE_CTRL_PROBE; > else > control->flags &=3D > - ~IEEE80211_TXCTL_RATE_CTRL_PROBE; > + ~IEEE80211_TXCTL_RATE_CTRL_PROBE > + ; ahem. > - printk(KERN_DEBUG "%s: ieee80211_beacon_get: no rate " > + printk(KERN_DEBUG > + "%s: ieee80211_beacon_get: no rate " > "found\n", local->mdev->name); Please change that to ... no " "rate found\n", ... instead. =20 > __le16 ieee80211_ctstoself_duration(struct ieee80211_hw *hw, > size_t frame_len, > - const struct ieee80211_tx_control *frame_txctl) > + const struct ieee80211_tx_control > + *frame_txctl) That's not how the kernel looks like in that case. I'd prefer this as __le16 ieee80211_ctstoself_duration( struct ieee80211_hw *hw, size_t frame_len, const struct ieee80211_tx_control *frame_txctl) > if (rx->sdata->bss) > - bss_tim_clear(rx->local, rx->sdata->bss, rx->sta->aid); > + bss_tim_clear(rx->local, rx->sdata->bss, > + rx->sta->aid); You have lots of space to indent it to the parenthesis. =20 > - if ((fc & IEEE80211_FCTL_FTYPE) !=3D (f_fc & IEEE80211_FCTL_FTYPE) || > + if ((fc & IEEE80211_FCTL_FTYPE) !=3D > + (f_fc & IEEE80211_FCTL_FTYPE) || same as above. > - (rx->fc & IEEE80211_FCTL_STYPE) =3D=3D IEEE80211_STYPE_PSPOLL))= && > + (rx->fc & IEEE80211_FCTL_STYPE) =3D=3D > + IEEE80211_STYPE_PSPOLL)) && ditto. > (rx->fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_DATA && > - (rx->fc & IEEE80211_FCTL_STYPE) !=3D IEEE80211_STYPE_NULLFUNC && > + (rx->fc & IEEE80211_FCTL_STYPE) !=3D > + IEEE80211_STYPE_NULLFUNC && ditto. > - (rx->fc & IEEE80211_FCTL_STYPE) !=3D IEEE80211_STYPE_NULLFUNC && > + (rx->fc & IEEE80211_FCTL_STYPE) !=3D > + IEEE80211_STYPE_NULLFUNC && ditto. =20 > @@ -3800,8 +3820,9 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct= sk_buff *skb, > continue; > rx.u.rx.ra_match =3D 0; > } else if (!multicast && > - compare_ether_addr(sdata->dev->dev_addr, > - hdr->addr1) !=3D 0) { > + compare_ether_addr( > + sdata->dev->dev_addr, > + hdr->addr1) !=3D 0) { > if (!sdata->promisc) > continue; Ugh. Somebody really needs to take this code and delete some indent by moving stuff into new functions. > if (net_ratelimit()) > - printk(KERN_DEBUG "%s: failed to copy " > + printk(KERN_DEBUG > + "%s: failed to copy " > "multicast frame for %s", > - local->mdev->name, prev->dev->name); > + local->mdev->name, > + prev->dev->name); Same as I said for that other message. johannes --=-NUhPxIb5q+tWW2XfJ0P9 Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Comment: Johannes Berg (powerbook) iD8DBQBGAYBP/ETPhpq3jKURAvMJAJ9NW2YoaOBvK8qK0jxKB9z2YZQxIACgmnxP 8x1rAX1QfsYrz3vQOgWH5EU= =/Sb2 -----END PGP SIGNATURE----- --=-NUhPxIb5q+tWW2XfJ0P9--