[PATCH] Fixed code errors and style. ie. trailing white spaces, spaces before brackets, 80+character lines, etc...

Joe Perches joe at perches.com
Wed Jun 2 02:51:48 UTC 2010


On Tue, 2010-06-01 at 19:19 -0700, harmonco at engr.orst.edu wrote:
> From: Cody Harmon <harmonco at flip.engr.oregonstate.edu>

Hi Cody.

Some of what you wrote are improvements, others not.
I only comment below on each issue once, though there
are multiple instances of each issue.

cheers, Joe

> diff --git a/drivers/staging/wlan-ng/p80211wext.c b/drivers/staging/wlan-ng/p80211wext.c
> index 387194d..d8d3db9 100644
> --- a/drivers/staging/wlan-ng/p80211wext.c
> +++ b/drivers/staging/wlan-ng/p80211wext.c
> @@ -232,9 +232,12 @@ struct iw_statistics *p80211wext_get_wireless_stats(netdevice_t *dev)
>  
>  	retval = wlandev->mlmerequest(wlandev, (p80211msg_t *) &quality);
>  
> -	wstats->qual.qual = qual_as_percent(quality.link.data);	/* overall link quality */
> -	wstats->qual.level = quality.level.data;	/* instant signal level */
> -	wstats->qual.noise = quality.noise.data;	/* instant noise level */
> +	/* overall link quality */
> +	wstats->qual.qual = qual_as_percent(quality.link.data);
> +	/* instant signal level */
> +	wstats->qual.level = quality.level.data;
> +	/* instant noise level */
> +	wstats->qual.noise = quality.noise.data;

Good
 
>  	wstats->qual.updated = IW_QUAL_ALL_UPDATED | IW_QUAL_DBM;
>  	wstats->discard.code = wlandev->rx.decrypt_err;
> @@ -287,8 +290,7 @@ static int p80211wext_giwfreq(netdevice_t *dev,
>  	unsigned int value;
>  
>  	result = p80211wext_getmib(wlandev,
> -				   DIDmib_dot11phy_dot11PhyDSSSTable_dot11CurrentChannel,
> -				   &value);
> +		DIDmib_dot11phy_dot11PhyDSSSTable_dot11CurrentChannel, &value);

not so good.  50+ character variable names really
are not easy to wrap at 80 chars and you probably
shouldn't bother.

[]

> @@ -419,7 +420,8 @@ static int p80211wext_giwrange(netdevice_t *dev,
>  	struct iw_range *range = (struct iw_range *)extra;
>  	int i, val;
>  
> -	/* for backward compatability set size and zero everything we don't understand */
> +	/* for backward compatability set size
> +	and zero everything we don't understand */

Comment blocks are not normally written like this

Normally this would be written something like:
	/*
	 * for backward compatibility set size and zero everything
	 * we don't understand
	 */
>  	data->length = sizeof(*range);
>  	memset(range, 0, sizeof(*range));
>  
> @@ -564,9 +566,9 @@ static int p80211wext_siwencode(netdevice_t *dev,
>  		/* Set current key number only if no keys are given */
>  		if (erq->flags & IW_ENCODE_NOKEY) {
>  			result =
> -				p80211wext_setmib(wlandev,
> -						  DIDmib_dot11smt_dot11PrivacyTable_dot11WEPDefaultKeyID,
> -						  i);
> +			p80211wext_setmib(wlandev,
> +			Dmib_dot11smt_dot11PrivacyTable_dot11WEPDefaultKeyID,
> +			i);

You end up with
			result =
			p80211wext_setmib(wlandev,
			[...]

which is not normally used.

I'd've written:
			result = p80211wext_setmib(wlandev,
						   DIDmib_dot11smt_dot11PrivacyTable_dot11WEPDefaultKeyID,
						   i);

and not cared about the 80 char warning

[]

> @@ -599,22 +602,22 @@ static int p80211wext_siwencode(netdevice_t *dev,
>  			switch (i) {
>  			case 0:
>  				pstr.did =
> -				    DIDmib_dot11smt_dot11WEPDefaultKeysTable_dot11WEPDefaultKey0;
> +					IDmib_dot11smt_dot11WEPDefaultKeysTable_dot11WEPDefaultKey0;
>  				break;

Remember to compile test before sending patches.





More information about the devel mailing list