Re: [Outreachy kernel] Re: [PATCH v2] staging: wlan-ng: Fix third argument going over 80 characters

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Sat, Mar 21, 2020 at 10:46:04AM +0100, Stefano Brivio wrote:
> On Sat, 21 Mar 2020 10:34:02 +0100
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Fri, Mar 20, 2020 at 06:39:05PM -0700, John B. Wyatt IV wrote:
> > > Create a new 'status' variable to store the value of a long argument
> > > that goes over 80 characters. The status variable is also used for
> > > an if check. Replacing that long statement in both places makes the
> > > code much easier to read.
> > > 
> > > Note: the status variable is assigned after a needed byte order
> > > conversion for usbin->rxfrm.desc.status, which uses a reference.
> > > 
> > > Issue reported by checkpatch.
> > > 
> > > Suggested-by: Stefano Brivio <sbrivio@redhat.com>
> > > Signed-off-by: John B. Wyatt IV <jbwyatt4@gmail.com>
> > > ---
> > > v2: Resubmitting after git rebase and comments from Julia Lawall 
> > > <julia.lawall@inria.fr>
> > > 
> > >  drivers/staging/wlan-ng/hfa384x_usb.c | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c
> > > index f8485601aead..9016e6b32efc 100644
> > > --- a/drivers/staging/wlan-ng/hfa384x_usb.c
> > > +++ b/drivers/staging/wlan-ng/hfa384x_usb.c
> > > @@ -3252,15 +3252,16 @@ static void hfa384x_usbin_rx(struct wlandevice *wlandev, struct sk_buff *skb)
> > >  	struct hfa384x *hw = wlandev->priv;
> > >  	int hdrlen;
> > >  	struct p80211_rxmeta *rxmeta;
> > > -	u16 data_len;
> > > -	u16 fc;
> > > +	u16 data_len, fc, status;  
> > 
> > No need to change data_len, or fc definitions, just add a new line here
> > instead for status.
> 
> My bad, I suggested that John would change that because to me:
> 
> 	u16 data_len;
> 	u16 fc;
> 	u16 status;
> 
> looks only slightly worse than:
> 
> 	u16 data_len;
> 	u16 fc;
> 
> but still it's horrible. If the idea is to avoid unrelated changes,
> yeah, I get it :)

The idea is to avoid unrelated changes :)