Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operator

From: Shreeya Patel <shreeya.patel23498@gmail.com>
To: Joe Perches <joe@perches.com>, Julia Lawall <julia.lawall@inria.fr>
Cc: Larry.Finger@lwfinger.net, gregkh@linuxfoundation.org,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	outreachy-kernel@googlegroups.com, sbrivio@redhat.com,
	daniel.baluta@gmail.com, nramas@linux.microsoft.com,
	hverkuil@xs4all.nl
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operator
Date: Mon, 09 Mar 2020 02:03:11 +0530	[thread overview]
Message-ID: <bbcb07684a546a1d4f5cd063e3914788d04bea6d.camel@gmail.com> (raw)
In-Reply-To: <cc0adb1f81afda526780fb217559a72031513909.camel@perches.com>

On Sun, 2020-03-08 at 13:19 -0700, Joe Perches wrote:
> On Mon, 2020-03-09 at 01:40 +0530, Shreeya Patel wrote:
> > On Sun, 2020-03-08 at 12:59 -0700, Joe Perches wrote:
> > Hi Joe,
> 
> Hello.
> []
> > > > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > > > > b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > > 
> > > []
> > > > > @@ -924,7 +924,7 @@ static void
> > > > > rtw_joinbss_update_network(struct
> > > > > adapter *padapter, struct wlan_net
> > > > >  	/* update fw_state will clr _FW_UNDER_LINKING here
> > > > > indirectly
> > > > > */
> > > > >  	switch (pnetwork->network.InfrastructureMode) {
> > > > >  	case Ndis802_11Infrastructure:
> > > > > -		if (pmlmepriv->fw_state&WIFI_UNDER_WPS)
> > > > > +		if (pmlmepriv->fw_state & WIFI_UNDER_WPS)
> > > > >  			pmlmepriv->fw_state =
> > > > > WIFI_STATION_STATE|WIFI_UNDER_WPS;
> > > 
> > > Like adding spaces around the | here too.
> > > 
> > 
> > I thought of doing this but then it was introducing another warning
> > of
> > "Line over 80 charachters" that is why I didn't proceed with it.
> 
> []
> > What is your suggestion over it? Should I let the line be over 80
> > characters and add spaces around the operators?
> 
> Just ignore the long line warnings, there are many already
> existing long
> lines in that subsystem.
> 
> That should be a style challenge for a later time.

Okay got it.

> 
> $ git ls-files drivers/staging/rtl8188eu | \
>   xargs awk '{ print length($0); }' | \
>   sort | uniq -c | sort -rn -k2
>       1 187
>       1 180
>       1 171
>       1 166
>       1 163
>       1 159
>       1 158
>       2 157
>       1 153
>       2 151
>       1 146
>       1 145
>       1 144
>       1 143
>       1 142
>       2 141
>       1 140
>       1 139
>       1 137
>       1 135
>       5 134
>       6 132
>       3 131
>       3 130
>       6 129
>       3 128
>       6 127
>       6 126
>       3 125
>       3 124
>       5 123
>       4 122
>       8 121
>       6 120
>       4 119
>       7 118
>      10 117
>      11 116
>       9 115
>       5 114
>      11 113
>      13 112
>       8 111
>      17 110
>      25 109
>      24 108
>      14 107
>      20 106
>      19 105
>      34 104
>      19 103
>      26 102
>      22 101
>      22 100
>      25 99
>      20 98
>      23 97
>      33 96
>      32 95
>      43 94
>      40 93
>      49 92
>      47 91
>      51 90
>      48 89
>      55 88
>      50 87
>      37 86
>      48 85
>      57 84
>      45 83
>      61 82
>      61 81

I think the idea of yours to automate the change for some common
warnings will be really helpful to get rid of them. 
Thanks, I'm going to try doing it for other drivers as well :)

> 
> 