2020-03-08 22:01:06

by Shreeya Patel

[permalink] [raw]
Subject: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operators

Add space around operators for improving the code
readability.

Reported by checkpatch.pl

Signed-off-by: Shreeya Patel <[email protected]>
---
drivers/staging/rtl8188eu/core/rtw_mlme.c | 40 +++++++++++------------
1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 8da955e8343b..9de2d421f6b1 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -149,7 +149,7 @@ static void _rtw_free_network(struct mlme_priv *pmlmepriv, struct wlan_network *
(check_fwstate(pmlmepriv, WIFI_ADHOC_STATE)))
lifetime = 1;
if (!isfreeall) {
- delta_time = (curr_time - pnetwork->last_scanned)/HZ;
+ delta_time = (curr_time - pnetwork->last_scanned) / HZ;
if (delta_time < lifetime)/* unit:sec */
return;
}
@@ -249,8 +249,8 @@ void rtw_generate_random_ibss(u8 *pibss)
pibss[1] = 0x11;
pibss[2] = 0x87;
pibss[3] = (u8)(curtime & 0xff);/* p[0]; */
- pibss[4] = (u8)((curtime>>8) & 0xff);/* p[1]; */
- pibss[5] = (u8)((curtime>>16) & 0xff);/* p[2]; */
+ pibss[4] = (u8)((curtime >> 8) & 0xff);/* p[1]; */
+ pibss[5] = (u8)((curtime >> 16) & 0xff);/* p[2]; */
}

u8 *rtw_get_capability_from_ie(u8 *ie)
@@ -357,9 +357,9 @@ void update_network(struct wlan_bssid_ex *dst, struct wlan_bssid_ex *src,
rssi_final = rssi_ori;
} else {
if (sq_smp != 101) { /* from the right channel */
- ss_final = ((u32)(src->PhyInfo.SignalStrength)+(u32)(dst->PhyInfo.SignalStrength)*4)/5;
- sq_final = ((u32)(src->PhyInfo.SignalQuality)+(u32)(dst->PhyInfo.SignalQuality)*4)/5;
- rssi_final = (src->Rssi+dst->Rssi*4)/5;
+ ss_final = ((u32)(src->PhyInfo.SignalStrength) + (u32)(dst->PhyInfo.SignalStrength) * 4) / 5;
+ sq_final = ((u32)(src->PhyInfo.SignalQuality) + (u32)(dst->PhyInfo.SignalQuality) * 4) / 5;
+ rssi_final = (src->Rssi + dst->Rssi * 4) / 5;
} else {
/* bss info not receiving from the right channel, use the original RX signal infos */
ss_final = dst->PhyInfo.SignalStrength;
@@ -510,7 +510,7 @@ static int rtw_is_desired_network(struct adapter *adapter, struct wlan_network *
privacy = pnetwork->network.Privacy;

if (check_fwstate(pmlmepriv, WIFI_UNDER_WPS)) {
- if (rtw_get_wps_ie(pnetwork->network.ies+_FIXED_IE_LENGTH_, pnetwork->network.ie_length-_FIXED_IE_LENGTH_, NULL, &wps_ielen))
+ if (rtw_get_wps_ie(pnetwork->network.ies + _FIXED_IE_LENGTH_, pnetwork->network.ie_length - _FIXED_IE_LENGTH_, NULL, &wps_ielen))
return true;
else
return false;
@@ -925,7 +925,7 @@ static void rtw_joinbss_update_network(struct adapter *padapter, struct wlan_net
switch (pnetwork->network.InfrastructureMode) {
case Ndis802_11Infrastructure:
if (pmlmepriv->fw_state & WIFI_UNDER_WPS)
- pmlmepriv->fw_state = WIFI_STATION_STATE|WIFI_UNDER_WPS;
+ pmlmepriv->fw_state = WIFI_STATION_STATE | WIFI_UNDER_WPS;
else
pmlmepriv->fw_state = WIFI_STATION_STATE;
break;
@@ -1097,14 +1097,14 @@ static u8 search_max_mac_id(struct adapter *padapter)
#if defined(CONFIG_88EU_AP_MODE)
if (check_fwstate(pmlmepriv, WIFI_AP_STATE)) {
for (aid = pstapriv->max_num_sta; aid > 0; aid--) {
- if (pstapriv->sta_aid[aid-1])
+ if (pstapriv->sta_aid[aid - 1])
break;
}
mac_id = aid + 1;
} else
#endif
{/* adhoc id = 31~2 */
- for (mac_id = NUM_STA-1; mac_id >= IBSS_START_MAC_ID; mac_id--) {
+ for (mac_id = NUM_STA - 1; mac_id >= IBSS_START_MAC_ID; mac_id--) {
if (pmlmeinfo->FW_sta_info[mac_id].status == 1)
break;
}
@@ -1123,7 +1123,7 @@ void rtw_stassoc_hw_rpt(struct adapter *adapter, struct sta_info *psta)

macid = search_max_mac_id(adapter);
rtw_hal_set_hwreg(adapter, HW_VAR_TX_RPT_MAX_MACID, (u8 *)&macid);
- media_status = (psta->mac_id<<8)|1; /* MACID|OPMODE:1 connect */
+ media_status = (psta->mac_id << 8) | 1; /* MACID|OPMODE:1 connect */
rtw_hal_set_hwreg(adapter, HW_VAR_H2C_MEDIA_STATUS_RPT, (u8 *)&media_status);
}

@@ -1213,7 +1213,7 @@ void rtw_stadel_event_callback(struct adapter *adapter, u8 *pbuf)
if (mac_id >= 0) {
u16 media_status;

- media_status = (mac_id<<8)|0; /* MACID|OPMODE:0 means disconnect */
+ media_status = (mac_id << 8) | 0; /* MACID|OPMODE:0 means disconnect */
/* for STA, AP, ADHOC mode, report disconnect stauts to FW */
rtw_hal_set_hwreg(adapter, HW_VAR_H2C_MEDIA_STATUS_RPT, (u8 *)&media_status);
}
@@ -1640,7 +1640,7 @@ int rtw_restruct_wmm_ie(struct adapter *adapter, u8 *in_ie, u8 *out_ie, uint in_
for (i = 12; i < in_len; i += (in_ie[i + 1] + 2) /* to the next IE element */) {
ielength = initial_out_len;

- if (in_ie[i] == 0xDD && in_ie[i+2] == 0x00 && in_ie[i+3] == 0x50 && in_ie[i+4] == 0xF2 && in_ie[i+5] == 0x02 && i+5 < in_len) {
+ if (in_ie[i] == 0xDD && in_ie[i + 2] == 0x00 && in_ie[i + 3] == 0x50 && in_ie[i + 4] == 0xF2 && in_ie[i + 5] == 0x02 && i + 5 < in_len) {
/* WMM element ID and OUI */
/* Append WMM IE to the last index of out_ie */

@@ -1734,13 +1734,13 @@ int rtw_restruct_sec_ie(struct adapter *adapter, u8 *in_ie, u8 *out_ie, uint in_
authmode = _WPA2_IE_ID_;

if (check_fwstate(pmlmepriv, WIFI_UNDER_WPS)) {
- memcpy(out_ie+ielength, psecuritypriv->wps_ie, psecuritypriv->wps_ie_len);
+ memcpy(out_ie + ielength, psecuritypriv->wps_ie, psecuritypriv->wps_ie_len);

ielength += psecuritypriv->wps_ie_len;
} else if ((authmode == _WPA_IE_ID_) || (authmode == _WPA2_IE_ID_)) {
/* copy RSN or SSN */
- memcpy(&out_ie[ielength], &psecuritypriv->supplicant_ie[0], psecuritypriv->supplicant_ie[1]+2);
- ielength += psecuritypriv->supplicant_ie[1]+2;
+ memcpy(&out_ie[ielength], &psecuritypriv->supplicant_ie[0], psecuritypriv->supplicant_ie[1] + 2);
+ ielength += psecuritypriv->supplicant_ie[1] + 2;
rtw_report_sec_ie(adapter, authmode, psecuritypriv->supplicant_ie);
}

@@ -1865,7 +1865,7 @@ unsigned int rtw_restructure_ht_ie(struct adapter *padapter, u8 *in_ie, u8 *out_

phtpriv->ht_option = false;

- p = rtw_get_ie(in_ie+12, _HT_CAPABILITY_IE_, &ielen, in_len-12);
+ p = rtw_get_ie(in_ie + 12, _HT_CAPABILITY_IE_, &ielen, in_len - 12);

if (p && ielen > 0) {
struct ieee80211_ht_cap ht_cap;
@@ -1904,16 +1904,16 @@ unsigned int rtw_restructure_ht_ie(struct adapter *padapter, u8 *in_ie, u8 *out_
else
ht_cap.ampdu_params_info |= IEEE80211_HT_CAP_AMPDU_DENSITY & 0x00;

- rtw_set_ie(out_ie+out_len, _HT_CAPABILITY_IE_,
+ rtw_set_ie(out_ie + out_len, _HT_CAPABILITY_IE_,
sizeof(struct ieee80211_ht_cap),
(unsigned char *)&ht_cap, pout_len);

phtpriv->ht_option = true;

- p = rtw_get_ie(in_ie+12, _HT_ADD_INFO_IE_, &ielen, in_len-12);
+ p = rtw_get_ie(in_ie + 12, _HT_ADD_INFO_IE_, &ielen, in_len - 12);
if (p && (ielen == sizeof(struct ieee80211_ht_addt_info))) {
out_len = *pout_len;
- rtw_set_ie(out_ie+out_len, _HT_ADD_INFO_IE_, ielen, p+2, pout_len);
+ rtw_set_ie(out_ie + out_len, _HT_ADD_INFO_IE_, ielen, p + 2, pout_len);
}
}
return phtpriv->ht_option;
--
2.17.1


2020-03-08 23:12:52

by Joe Perches

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operators

On Mon, 2020-03-09 at 03:30 +0530, Shreeya Patel wrote:
> Add space around operators for improving the code
> readability.

Hello again Shreeya.

The subject isn't really quite appropriate as you
are not doing this space around operator addition
for the entire subsystem.

IMO, the subject should be:

[PATCH] staging: rtl8188eu: rtw_mlme: Add spaces around operators

because you are only performing this change on this
single file.

If you were to do this for every single file in the
subsystem, you could have many individual patches with
the exact same subject line.

And it would be good to show in the changelog that you
have compiled the file pre and post patch without object
code change.

Also, it's good to show that git diff -w shows no source
file changes.

> Reported by checkpatch.pl
>
> Signed-off-by: Shreeya Patel <[email protected]>
> ---
> drivers/staging/rtl8188eu/core/rtw_mlme.c | 40 +++++++++++------------
> 1 file changed, 20 insertions(+), 20 deletions(-)

When I try this using checkpatch --fix-inplace, I get
21 changes against the latest -next tree.

What tree are you doing this against?


2020-03-09 05:49:43

by Shreeya Patel

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operators

On Sun, 2020-03-08 at 16:05 -0700, Joe Perches wrote:
> On Mon, 2020-03-09 at 03:30 +0530, Shreeya Patel wrote:
> > Add space around operators for improving the code
> > readability.
>
> Hello again Shreeya.
>
> The subject isn't really quite appropriate as you
> are not doing this space around operator addition
> for the entire subsystem.
>
> IMO, the subject should be:
>
> [PATCH] staging: rtl8188eu: rtw_mlme: Add spaces around operators
>
> because you are only performing this change on this
> single file.
>
> If you were to do this for every single file in the
> subsystem, you could have many individual patches with
> the exact same subject line.

Oh yes, thanks for correcting me.
>
> And it would be good to show in the changelog that you
> have compiled the file pre and post patch without object
> code change.
>
> Also, it's good to show that git diff -w shows no source
> file changes.

okay will do this in v2.

>
> > Reported by checkpatch.pl
> >
> > Signed-off-by: Shreeya Patel <[email protected]>
> > ---
> > drivers/staging/rtl8188eu/core/rtw_mlme.c | 40 +++++++++++------
> > ------
> > 1 file changed, 20 insertions(+), 20 deletions(-)
>
> When I try this using checkpatch --fix-inplace, I get
> 21 changes against the latest -next tree.
>
> What tree are you doing this against?

I am doing this against the latest -testing tree

Thanks
>
>

2020-03-09 06:45:32

by Shreeya Patel

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operators

On Sun, 2020-03-08 at 16:05 -0700, Joe Perches wrote:
> On Mon, 2020-03-09 at 03:30 +0530, Shreeya Patel wrote:
> > Add space around operators for improving the code
> > readability.
>
> Hello again Shreeya.
>
I have some questions here...

> The subject isn't really quite appropriate as you
> are not doing this space around operator addition
> for the entire subsystem.
>
> IMO, the subject should be:
>
> [PATCH] staging: rtl8188eu: rtw_mlme: Add spaces around operators
>
> because you are only performing this change on this
> single file.
>
> If you were to do this for every single file in the
> subsystem, you could have many individual patches with
> the exact same subject line.
>
> And it would be good to show in the changelog that you
> have compiled the file pre and post patch without object
> code change.
>
I'm not sure how to show this. Do you mean to add the output of
"make drivers/staging/rtl8188eu/core" before and after the changes?

I also don't understand the meaning of no object code change. If we are
making the changes to code and then compiling it using the make command
then a new file with .o extension is created and replaced by the
previous one isn't it?

> Also, it's good to show that git diff -w shows no source
> file changes.
>

And this has to be...
git diff -w --shortstat drivers/staging/rtl8188eu/core/

Am I correct?

Thanks

> > Reported by checkpatch.pl
> >
> > Signed-off-by: Shreeya Patel <[email protected]>
> > ---
> > drivers/staging/rtl8188eu/core/rtw_mlme.c | 40 +++++++++++------
> > ------
> > 1 file changed, 20 insertions(+), 20 deletions(-)
>
> When I try this using checkpatch --fix-inplace, I get
> 21 changes against the latest -next tree.
>
> What tree are you doing this against?
>
>

2020-03-09 07:37:07

by Julia Lawall

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operators

On Mon, 9 Mar 2020, Shreeya Patel wrote:

> On Sun, 2020-03-08 at 16:05 -0700, Joe Perches wrote:
> > On Mon, 2020-03-09 at 03:30 +0530, Shreeya Patel wrote:
> > > Add space around operators for improving the code
> > > readability.
> >
> > Hello again Shreeya.
> >
> I have some questions here...
>
> > The subject isn't really quite appropriate as you
> > are not doing this space around operator addition
> > for the entire subsystem.
> >
> > IMO, the subject should be:
> >
> > [PATCH] staging: rtl8188eu: rtw_mlme: Add spaces around operators
> >
> > because you are only performing this change on this
> > single file.
> >
> > If you were to do this for every single file in the
> > subsystem, you could have many individual patches with
> > the exact same subject line.
> >
> > And it would be good to show in the changelog that you
> > have compiled the file pre and post patch without object
> > code change.
> >
> I'm not sure how to show this. Do you mean to add the output of
> "make drivers/staging/rtl8188eu/core" before and after the changes?

You are working on one specific file, maybe foo.c. Compile before
making changes, which will give you foo.o. Rename that file to something
else. Make your changes and compile again. Do a diff with the previously
compiled file. It should produce nothing, indicating no difference.

If this .o file doesn't change and you only changed this .c file, the
whole compiled driver won't change either.

> I also don't understand the meaning of no object code change. If we are
> making the changes to code and then compiling it using the make command
> then a new file with .o extension is created and replaced by the
> previous one isn't it?
>
> > Also, it's good to show that git diff -w shows no source
> > file changes.
> >
>
> And this has to be...
> git diff -w --shortstat drivers/staging/rtl8188eu/core/

--shortstat does not seem useful. What you hope to see is that it
produces nothing.

julia

> Am I correct?
>
> Thanks
>
> > > Reported by checkpatch.pl
> > >
> > > Signed-off-by: Shreeya Patel <[email protected]>
> > > ---
> > > drivers/staging/rtl8188eu/core/rtw_mlme.c | 40 +++++++++++------
> > > ------
> > > 1 file changed, 20 insertions(+), 20 deletions(-)
> >
> > When I try this using checkpatch --fix-inplace, I get
> > 21 changes against the latest -next tree.
> >
> > What tree are you doing this against?
> >
> >
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/af1a27fb8c5f7efbaf99ce3055cf3801b366d627.camel%40gmail.com.
>

2020-03-09 13:26:45

by Shreeya Patel

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operators

On Mon, 2020-03-09 at 08:35 +0100, Julia Lawall wrote:
> On Mon, 9 Mar 2020, Shreeya Patel wrote:
>
> > On Sun, 2020-03-08 at 16:05 -0700, Joe Perches wrote:
> > > On Mon, 2020-03-09 at 03:30 +0530, Shreeya Patel wrote:
> > > > Add space around operators for improving the code
> > > > readability.
> > >
> > > Hello again Shreeya.
> > >
> >
> > I have some questions here...
> >
> > > The subject isn't really quite appropriate as you
> > > are not doing this space around operator addition
> > > for the entire subsystem.
> > >
> > > IMO, the subject should be:
> > >
> > > [PATCH] staging: rtl8188eu: rtw_mlme: Add spaces around operators
> > >
> > > because you are only performing this change on this
> > > single file.
> > >
> > > If you were to do this for every single file in the
> > > subsystem, you could have many individual patches with
> > > the exact same subject line.
> > >
> > > And it would be good to show in the changelog that you
> > > have compiled the file pre and post patch without object
> > > code change.
> > >
> >
> > I'm not sure how to show this. Do you mean to add the output of
> > "make drivers/staging/rtl8188eu/core" before and after the changes?
>
> You are working on one specific file, maybe foo.c. Compile before
> making changes, which will give you foo.o. Rename that file to
> something
> else. Make your changes and compile again. Do a diff with the
> previously
> compiled file. It should produce nothing, indicating no difference.
>
> If this .o file doesn't change and you only changed this .c file, the
> whole compiled driver won't change either.
>

ok, got it.

> > I also don't understand the meaning of no object code change. If we
> > are
> > making the changes to code and then compiling it using the make
> > command
> > then a new file with .o extension is created and replaced by the
> > previous one isn't it?
> >
> > > Also, it's good to show that git diff -w shows no source
> > > file changes.
> > >
> >
> > And this has to be...
> > git diff -w --shortstat drivers/staging/rtl8188eu/core/
>
> --shortstat does not seem useful. What you hope to see is that it
> produces nothing.
>
Okay.
I will send a V2 with all the changes required.

Btw Joe, I am working against staging-testing tree

> julia
>
> > Am I correct?
> >
> > Thanks
> >
> > > > Reported by checkpatch.pl
> > > >
> > > > Signed-off-by: Shreeya Patel <[email protected]>
> > > > ---
> > > > drivers/staging/rtl8188eu/core/rtw_mlme.c | 40 +++++++++++--
> > > > ----
> > > > ------
> > > > 1 file changed, 20 insertions(+), 20 deletions(-)
> > >
> > > When I try this using checkpatch --fix-inplace, I get
> > > 21 changes against the latest -next tree.
> > >
> > > What tree are you doing this against?
> > >
> > >
> >
> > --
> > You received this message because you are subscribed to the Google
> > Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> > send an email to [email protected].
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/outreachy-kernel/af1a27fb8c5f7efbaf99ce3055cf3801b366d627.camel%40gmail.com
> > .
> >