linux-kernel - Re: [PATCHv3 02/15] staging: rtl8192u: r8192U_core: add temporary variables to keep lines under 80 characters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150921021600.GB10943@kroah.com>
Date:	Sun, 20 Sep 2015 19:16:00 -0700
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	Raphaël Beamonte <raphael.beamonte@...il.com>
Cc:	Cristina Opriceana <cristina.opriceana@...il.com>,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv3 02/15] staging: rtl8192u: r8192U_core: add temporary
 variables to keep lines under 80 characters

On Sun, Sep 20, 2015 at 01:14:14PM -0400, Raphaël Beamonte wrote:
> Add some temporary variables to reduce line length under the maximum
> of 80 characters, as per the kernel code style.
> 
> Signed-off-by: Raphaël Beamonte <raphael.beamonte@...il.com>
> ---
>  drivers/staging/rtl8192u/r8192U_core.c | 139 ++++++++++++++++++++++-----------
>  1 file changed, 94 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index 28b54ba..2abc3e77 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -171,6 +171,7 @@ static void rtl819x_set_channel_map(u8 channel_plan, struct r8192_priv *priv)
>  {
>  	int i, max_chan = -1, min_chan = -1;
>  	struct ieee80211_device *ieee = priv->ieee80211;
> +	struct CHANNEL_LIST cl;

A whole structure on the stack?  Are you sure you want to do that?

>  
>  	switch (channel_plan) {
>  	case COUNTRY_CODE_FCC:
> @@ -194,15 +195,18 @@ static void rtl819x_set_channel_map(u8 channel_plan, struct r8192_priv *priv)
>  				 "unknown rf chip, can't set channel map in function:%s()\n",
>  				 __func__);
>  		}
> -		if (ChannelPlan[channel_plan].Len != 0) {
> +		cl = ChannelPlan[channel_plan];

You just did a memory copy of a whole structure, did you really want to
do that?  If so, why?  You just changed the logic of this function,
potentially slowing things down and setting yourself up for causing real
bugs (hint, anything you now change in that structure is not going to be
ever saved, it will go away after the function is exited.)

Please be more aware of how structures and pointers work in C before
doing changes like this, I can't take this change.

> +		if (cl.Len != 0) {
>  			/* Clear old channel map */
>  			memset(GET_DOT11D_INFO(ieee)->channel_map, 0,
>  			       sizeof(GET_DOT11D_INFO(ieee)->channel_map));
>  			/* Set new channel map */
> -			for (i = 0; i < ChannelPlan[channel_plan].Len; i++) {
> -				if (ChannelPlan[channel_plan].Channel[i] < min_chan || ChannelPlan[channel_plan].Channel[i] > max_chan)
> +			for (i = 0; i < cl.Len; i++) {
> +				u8 chan = cl.Channel[i];
> +
> +				if (chan < min_chan || chan > max_chan)
>  					break;
> -				GET_DOT11D_INFO(ieee)->channel_map[ChannelPlan[channel_plan].Channel[i]] = 1;
> +				GET_DOT11D_INFO(ieee)->channel_map[chan] = 1;
>  			}
>  		}
>  		break;
> @@ -1649,9 +1653,12 @@ short rtl8192_tx(struct net_device *dev, struct sk_buff *skb)
>  					  &zero, 0, tx_zero_isr, dev);
>  			status = usb_submit_urb(tx_urb_zero, GFP_ATOMIC);
>  			if (status) {
> +				atomic_t tx =
> +					priv->tx_pending[tcb_desc->queue_index];

Oh that's funny, think about what you just did here.

Please get some more experience with C before doing kernel development.
C is tricky and will let you shoot yourself in the foot and any other
body part if you are not _VERY_ careful.  You just blew off a few limbs
without realizing it at all in just the first two changes you made.

thanks,

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