Re: [PATCH] Staging: wlan-ng: fixed multiple styling issues inprism2sta.c This is a patch to the prism2sta.c file that fixesmultiple warnings generated by the checkpatch.pl tool, mostlyrelated to the 80 character limit, though there were also a fewrelated to having a space following the & operator. code has beenrefactored and no longer generates errors Signed-off-by: Jim Dog<jimdog@northern-indymedia.org>

From: Dan Carpenter
Date: Thu Feb 25 2010 - 08:12:27 EST

On Thu, Feb 25, 2010 at 12:36:52PM +0000, Jim Dog wrote:
> ---
> drivers/staging/wlan-ng/prism2sta.c | 74 +++++++++++++++++++----------------
> 1 files changed, 40 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/staging/wlan-ng/prism2sta.c b/drivers/staging/wlan-ng/prism2sta.c
> index 50f301d..871d8f6 100644
> --- a/drivers/staging/wlan-ng/prism2sta.c
> +++ b/drivers/staging/wlan-ng/prism2sta.c
> @@ -64,7 +64,7 @@
> #include <linux/byteorder/generic.h>
> #include <linux/ctype.h>
>
> -#include <asm/io.h>
> +#include <linux/io.h>
> #include <linux/delay.h>
> #include <asm/byteorder.h>
> #include <linux/if_arp.h>
> @@ -455,10 +455,10 @@ u32 prism2sta_ifstate(wlandevice_t *wlandev, u32 ifstate)
> result = hfa384x_drvr_start(hw);
> if (result) {
> printk(KERN_ERR
> - "hfa384x_drvr_start() failed,"
> - "result=%d\n", (int)result);
> + "hfa384x_drvr_start() failed,"
> + "result=%d\n", (int)result);
> result =
> - P80211ENUM_resultcode_implementation_failure;
> + P80211ENUM_resultcode_implementation_failure;
> wlandev->msdstate = WLAN_MSD_HWPRESENT;
> break;
> }
> @@ -503,7 +503,7 @@ u32 prism2sta_ifstate(wlandevice_t *wlandev, u32 ifstate)
> "hfa384x_drvr_start() failed,"
> "result=%d\n", (int)result);
> result =
> - P80211ENUM_resultcode_implementation_failure;
> + P80211ENUM_resultcode_implementation_failure;
> wlandev->msdstate = WLAN_MSD_HWPRESENT;
> break;
> }
> @@ -514,7 +514,7 @@ u32 prism2sta_ifstate(wlandevice_t *wlandev, u32 ifstate)
> "prism2sta_getcardinfo() failed,"
> "result=%d\n", (int)result);
> result =
> - P80211ENUM_resultcode_implementation_failure;
> + P80211ENUM_resultcode_implementation_failure;
> hfa384x_drvr_stop(hw);
> wlandev->msdstate = WLAN_MSD_HWPRESENT;
> break;
> @@ -525,7 +525,7 @@ u32 prism2sta_ifstate(wlandevice_t *wlandev, u32 ifstate)
> "prism2sta_globalsetup() failed,"
> "result=%d\n", (int)result);
> result =
> - P80211ENUM_resultcode_implementation_failure;
> + P80211ENUM_resultcode_implementation_failure;
> hfa384x_drvr_stop(hw);
> wlandev->msdstate = WLAN_MSD_HWPRESENT;
> break;
> @@ -1023,13 +1023,13 @@ static void prism2sta_inf_tallies(wlandevice_t *wlandev,
>
> cnt = sizeof(hfa384x_CommTallies32_t) / sizeof(u32);
> if (inf->framelen > 22) {
> - dst = (u32 *) & hw->tallies;
> - src32 = (u32 *) & inf->info.commtallies32;
> + dst = (u32 *) &hw->tallies;
> + src32 = (u32 *) &inf->info.commtallies32;
> for (i = 0; i < cnt; i++, dst++, src32++)
> *dst += le32_to_cpu(*src32);
> } else {
> - dst = (u32 *) & hw->tallies;
> - src16 = (u16 *) & inf->info.commtallies16;
> + dst = (u32 *) &hw->tallies;
> + src16 = (u16 *) &inf->info.commtallies16;
> for (i = 0; i < cnt; i++, dst++, src16++)
> *dst += le16_to_cpu(*src16);
> }
> @@ -1179,7 +1179,8 @@ static void prism2sta_inf_chinforesults(wlandevice_t *wlandev,
> le16_to_cpu(inf->info.chinforesult.result[n].
> active);
> pr_debug
> - ("chinfo: channel %d, %s level (avg/peak)=%d/%d dB, pcf %d\n",
> + ("chinfo:
> + channel %d, %s level (avg/peak)=%d/%d dB, pcf %d\n",

You introduced a bug here.

I want to encourage newbies but really these patches are the wrong
idea.

We could easily do this with a script. It is scripts that should be
slaving away for humans and not the other way round. In the case of
handling string literals, you're slaving away to do what a _buggy_
version of checkpatch.pl tells you to do.

It's not that hard to find actual bugs to fix. Here is a list of 11
buffer overflows I posted earlier.
http://lkml.org/lkml/2010/2/25/38

If you think about it, it's better to start with the goal of fixing an
actual bug. Otherwise you have the possibility of introducing a bug,
but no possibility of fixing a bug.

Anyway. Aim higher is all I'm saying. Stop with the checkpatch.pl.

regards,
dan carpenter

> channel + 1,
> chinforesult->
> active & HFA384x_CHINFORESULT_BSSACTIVE ? "signal"
> @@ -1246,7 +1247,8 @@ void prism2sta_processing_defer(struct work_struct *data)
>
> netif_carrier_on(wlandev->netdev);
>
> - /* If we are joining a specific AP, set our state and reset retries */
> + /* If we are joining a specific AP, */
> + /* set our state and reset retries */
> if (hw->join_ap == 1)
> hw->join_ap = 2;
> hw->join_retries = 60;
> @@ -1260,10 +1262,10 @@ void prism2sta_processing_defer(struct work_struct *data)
> /* For non-usb devices, we can use the sync versions */
> /* Collect the BSSID, and set state to allow tx */
>
> - result = hfa384x_drvr_getconfig(hw,
> - HFA384x_RID_CURRENTBSSID,
> - wlandev->bssid,
> - WLAN_BSSID_LEN);
> + result = hfa384x_drvr_getconfig
> + (hw, HFA384x_RID_CURRENTBSSID,
> + wlandev->bssid,
> + WLAN_BSSID_LEN);
> if (result) {
> pr_debug
> ("getconfig(0x%02x) failed, result = %d\n",
> @@ -1280,14 +1282,14 @@ void prism2sta_processing_defer(struct work_struct *data)
> HFA384x_RID_CURRENTSSID, result);
> goto failed;
> }
> - prism2mgmt_bytestr2pstr((hfa384x_bytestr_t *) & ssid,
> + prism2mgmt_bytestr2pstr((hfa384x_bytestr_t *) &ssid,
> (p80211pstrd_t *) &
> wlandev->ssid);
>
> /* Collect the port status */
> - result = hfa384x_drvr_getconfig16(hw,
> - HFA384x_RID_PORTSTATUS,
> - &portstatus);
> + result = hfa384x_drvr_getconfig16
> + (hw, HFA384x_RID_PORTSTATUS,
> + &portstatus);
> if (result) {
> pr_debug
> ("getconfig(0x%02x) failed, result = %d\n",
> @@ -1322,7 +1324,7 @@ void prism2sta_processing_defer(struct work_struct *data)
> &joinreq,
> HFA384x_RID_JOINREQUEST_LEN);
> printk(KERN_INFO
> - "linkstatus=DISCONNECTED (re-submitting join)\n");
> + "linkstatus=DISCONNECTED (re-submitting join)\n");
> } else {
> if (wlandev->netdev->type == ARPHRD_ETHER)
> printk(KERN_INFO
> @@ -1368,8 +1370,8 @@ void prism2sta_processing_defer(struct work_struct *data)
> HFA384x_RID_CURRENTSSID, result);
> goto failed;
> }
> - prism2mgmt_bytestr2pstr((hfa384x_bytestr_t *) & ssid,
> - (p80211pstrd_t *) & wlandev->ssid);
> + prism2mgmt_bytestr2pstr((hfa384x_bytestr_t *) &ssid,
> + (p80211pstrd_t *) &wlandev->ssid);
>
> hw->link_status = HFA384x_LINK_CONNECTED;
> netif_carrier_on(wlandev->netdev);
> @@ -1509,14 +1511,15 @@ static void prism2sta_inf_assocstatus(wlandevice_t *wlandev,
> rec.reason = le16_to_cpu(rec.reason);
>
> /*
> - ** Find the address in the list of authenticated stations. If it wasn't
> + ** Find the address in the list of authenticated stations. If it wasn't
> ** found, then this address has not been previously authenticated and
> ** something weird has happened if this is anything other than an
> ** "authentication failed" message. If the address was found, then
> ** set the "associated" flag for that station, based on whether the
> - ** station is associating or losing its association. Something weird
> - ** has also happened if we find the address in the list of authenticated
> - ** stations but we are getting an "authentication failed" message.
> + ** station is associating or losing its association. Something weird
> + ** has also happened if we find the address in the list of
> + ** authenticated stations but we are getting an "authentication failed"
> + ** message.
> */
>
> for (i = 0; i < hw->authlist.cnt; i++)
> @@ -1526,7 +1529,8 @@ static void prism2sta_inf_assocstatus(wlandevice_t *wlandev,
> if (i >= hw->authlist.cnt) {
> if (rec.assocstatus != HFA384x_ASSOCSTATUS_AUTHFAIL)
> printk(KERN_WARNING
> - "assocstatus info frame received for non-authenticated station.\n");
> + "assocstatus info frame received
> + for non-authenticated station.\n");
> } else {
> hw->authlist.assoc[i] =
> (rec.assocstatus == HFA384x_ASSOCSTATUS_STAASSOC ||
> @@ -1534,7 +1538,8 @@ static void prism2sta_inf_assocstatus(wlandevice_t *wlandev,
>
> if (rec.assocstatus == HFA384x_ASSOCSTATUS_AUTHFAIL)
> printk(KERN_WARNING
> - "authfail assocstatus info frame received for authenticated station.\n");
> + "authfail assocstatus info frame received
> + for authenticated station.\n");
> }
>
> return;
> @@ -1682,7 +1687,7 @@ static void prism2sta_inf_authreq_defer(wlandevice_t *wlandev,
>
> /*
> ** If the authentication is okay, then add the MAC address to the list
> - ** of authenticated stations. Don't add the address if it is already in
> + ** of authenticated stations. Don't add the address if it's already in
> ** the list. (802.11b does not seem to disallow a station from issuing
> ** an authentication request when the station is already authenticated.
> ** Does this sort of thing ever happen? We might as well do the check
> @@ -1997,7 +2002,8 @@ void prism2sta_commsqual_defer(struct work_struct *data)
> if (wlandev->macmode != WLAN_MACMODE_IBSS_STA) {
> result = hfa384x_drvr_getconfig(hw, HFA384x_RID_DBMCOMMSQUALITY,
> &hw->qual,
> - HFA384x_RID_DBMCOMMSQUALITY_LEN);
> + HFA384x_RID_DBMCOMMSQUALITY_LEN)
> + ;
>
> if (result) {
> printk(KERN_ERR "error fetching commsqual\n");
> @@ -2028,8 +2034,8 @@ void prism2sta_commsqual_defer(struct work_struct *data)
> HFA384x_RID_CURRENTSSID, result);
> goto done;
> }
> - prism2mgmt_bytestr2pstr((hfa384x_bytestr_t *) & ssid,
> - (p80211pstrd_t *) & wlandev->ssid);
> + prism2mgmt_bytestr2pstr((hfa384x_bytestr_t *) &ssid,
> + (p80211pstrd_t *) &wlandev->ssid);
>
> /* Reschedule timer */
> mod_timer(&hw->commsqual_timer, jiffies + HZ);
> --
> 1.6.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/