2022-02-16 05:56:35

by Phillip Potter

[permalink] [raw]
Subject: [PATCH v2 15/15] staging: r8188eu: correct long line warnings near prior DBG_88E calls

Where it is possible (without out-of-patch-series-scope large scale
refactoring), correct code to remove checkpatch warnings about lines
that are too long, also correcting operator spacing where appropriate
for these lines as well. These warnings occur mostly due to so many
DBG_88E removals and parentheses tweaks etc. being adjacent to such
long lines, which are therefore included in the resultant diff.

Signed-off-by: Phillip Potter <[email protected]>
---
drivers/staging/r8188eu/core/rtw_br_ext.c | 12 +++++++++---
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 3 ++-
drivers/staging/r8188eu/core/rtw_p2p.c | 15 ++++++++++++---
drivers/staging/r8188eu/core/rtw_pwrctrl.c | 3 ++-
drivers/staging/r8188eu/core/rtw_wlan_util.c | 6 ++++--
5 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
index ddc3a2c8aaca..d68611ef22f8 100644
--- a/drivers/staging/r8188eu/core/rtw_br_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
@@ -382,7 +382,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
if (protocol == ETH_P_IP) {
struct iphdr *iph = (struct iphdr *)(skb->data + ETH_HLEN);

- if (((unsigned char *)(iph) + (iph->ihl<<2)) >= (skb->data + ETH_HLEN + skb->len))
+ if (((unsigned char *)(iph) + (iph->ihl << 2)) >= (skb->data + ETH_HLEN + skb->len))
return -1;

switch (method) {
@@ -451,7 +451,11 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
pOldTag = (struct pppoe_tag *)__nat25_find_pppoe_tag(ph, ntohs(PTT_RELAY_SID));
if (pOldTag) { /* if SID existed, copy old value and delete it */
old_tag_len = ntohs(pOldTag->tag_len);
- if (old_tag_len+TAG_HDR_LEN+MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN > sizeof(tag_buf))
+ if (old_tag_len +
+ TAG_HDR_LEN +
+ MAGIC_CODE_LEN +
+ RTL_RELAY_TAG_LEN >
+ sizeof(tag_buf))
return -1;

memcpy(tag->tag_data+MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN,
@@ -476,7 +480,9 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
return -1;
} else { /* not add relay tag */
if (priv->pppoe_connection_in_progress &&
- memcmp(skb->data+ETH_ALEN, priv->pppoe_addr, ETH_ALEN))
+ memcmp(skb->data + ETH_ALEN,
+ priv->pppoe_addr,
+ ETH_ALEN))
return -2;

if (priv->pppoe_connection_in_progress == 0)
diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 2b6dbabe47c8..0d9351903ae5 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -3583,7 +3583,8 @@ static s32 rtw_action_public_decache(struct recv_frame *recv_frame, s32 token)

if (GetRetry(frame)) {
if (token >= 0) {
- if ((seq_ctrl == mlmeext->action_public_rxseq) && (token == mlmeext->action_public_dialog_token))
+ if ((seq_ctrl == mlmeext->action_public_rxseq) &&
+ (token == mlmeext->action_public_dialog_token))
return _FAIL;
} else {
if (seq_ctrl == mlmeext->action_public_rxseq)
diff --git a/drivers/staging/r8188eu/core/rtw_p2p.c b/drivers/staging/r8188eu/core/rtw_p2p.c
index 9467a5dcc990..48500fb82250 100644
--- a/drivers/staging/r8188eu/core/rtw_p2p.c
+++ b/drivers/staging/r8188eu/core/rtw_p2p.c
@@ -1154,7 +1154,8 @@ u8 process_p2p_group_negotation_req(struct wifidirect_info *pwdinfo, u8 *pframe,
peer_operating_ch = operatingch_info[4];

if (rtw_p2p_is_channel_list_ok(peer_operating_ch,
- ch_list_inclusioned, ch_num_inclusioned))
+ ch_list_inclusioned,
+ ch_num_inclusioned))
/**
* Change our operating channel as peer's for compatibility.
*/
@@ -1270,7 +1271,11 @@ u8 process_p2p_group_negotation_resp(struct wifidirect_info *pwdinfo, u8 *pframe
/* Try to get the operation channel information */

attr_contentlen = 0;
- if (rtw_get_p2p_attr_content(p2p_ie, p2p_ielen, P2P_ATTR_OPERATING_CH, operatingch_info, &attr_contentlen))
+ if (rtw_get_p2p_attr_content(p2p_ie,
+ p2p_ielen,
+ P2P_ATTR_OPERATING_CH,
+ operatingch_info,
+ &attr_contentlen))
pwdinfo->peer_operating_ch = operatingch_info[4];

/* Try to get the channel list information */
@@ -1377,7 +1382,11 @@ u8 process_p2p_group_negotation_confirm(struct wifidirect_info *pwdinfo, u8 *pfr
}

attr_contentlen = 0;
- if (rtw_get_p2p_attr_content(p2p_ie, p2p_ielen, P2P_ATTR_OPERATING_CH, operatingch_info, &attr_contentlen))
+ if (rtw_get_p2p_attr_content(p2p_ie,
+ p2p_ielen,
+ P2P_ATTR_OPERATING_CH,
+ operatingch_info,
+ &attr_contentlen))
pwdinfo->peer_operating_ch = operatingch_info[4];

/* Get the next P2P IE */
diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index c7c79cd9e213..b20436f75459 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -171,7 +171,8 @@ static u8 PS_RDY_CHECK(struct adapter *padapter)
return false;
if (pwrpriv->bInSuspend)
return false;
- if (padapter->securitypriv.dot11AuthAlgrthm == dot11AuthAlgrthm_8021X && !padapter->securitypriv.binstallGrpkey)
+ if (padapter->securitypriv.dot11AuthAlgrthm == dot11AuthAlgrthm_8021X &&
+ !padapter->securitypriv.binstallGrpkey)
return false;
return true;
}
diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
index e8bf5672ff6c..33c0228204ad 100644
--- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
@@ -926,7 +926,8 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8 *pframe, u32 packet_len)

if (memcmp(bssid->Ssid.Ssid, cur_network->network.Ssid.Ssid, 32) ||
bssid->Ssid.SsidLength != cur_network->network.Ssid.SsidLength) {
- if (bssid->Ssid.Ssid[0] != '\0' && bssid->Ssid.SsidLength != 0) /* not hidden ssid */
+ /* not hidden ssid */
+ if (bssid->Ssid.Ssid[0] != '\0' && bssid->Ssid.SsidLength != 0)
goto _mismatch;
}

@@ -966,7 +967,8 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8 *pframe, u32 packet_len)
rtw_parse_wpa2_ie(pbuf, wpa_ielen + 2, &group_cipher, &pairwise_cipher, &is_8021x);
}

- if (pairwise_cipher != cur_network->BcnInfo.pairwise_cipher || group_cipher != cur_network->BcnInfo.group_cipher)
+ if (pairwise_cipher != cur_network->BcnInfo.pairwise_cipher ||
+ group_cipher != cur_network->BcnInfo.group_cipher)
goto _mismatch;

if (is_8021x != cur_network->BcnInfo.is_8021x)
--
2.34.1


2022-02-16 10:03:29

by David Laight

[permalink] [raw]
Subject: RE: [PATCH v2 15/15] staging: r8188eu: correct long line warnings near prior DBG_88E calls

From: Phillip Potter
> Sent: 16 February 2022 01:07
>
> Where it is possible (without out-of-patch-series-scope large scale
> refactoring), correct code to remove checkpatch warnings about lines
> that are too long, also correcting operator spacing where appropriate
> for these lines as well. These warnings occur mostly due to so many
> DBG_88E removals and parentheses tweaks etc. being adjacent to such
> long lines, which are therefore included in the resultant diff.
...

Somewhere my copy of this seems to have got its tabs deleted.
I blame outlook :-)

> diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> index ddc3a2c8aaca..d68611ef22f8 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -382,7 +382,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> if (protocol == ETH_P_IP) {
> struct iphdr *iph = (struct iphdr *)(skb->data + ETH_HLEN);
>
> -if (((unsigned char *)(iph) + (iph->ihl<<2)) >= (skb->data + ETH_HLEN + skb->len))
> +if (((unsigned char *)(iph) + (iph->ihl << 2)) >= (skb->data + ETH_HLEN + skb->len))

You can delete at least three sets of () from that line.

> return -1;
>
> switch (method) {
> @@ -451,7 +451,11 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> pOldTag = (struct pppoe_tag *)__nat25_find_pppoe_tag(ph, ntohs(PTT_RELAY_SID));
> if (pOldTag) { /* if SID existed, copy old value and delete it */
> old_tag_len = ntohs(pOldTag->tag_len);
> -if (old_tag_len+TAG_HDR_LEN+MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN > sizeof(tag_buf))
> +if (old_tag_len +
> + TAG_HDR_LEN +
> + MAGIC_CODE_LEN +
> + RTL_RELAY_TAG_LEN >
> + sizeof(tag_buf))
> return -1;

That change really doesn't help readability at all.
There isn't much point shortening it that much like that, especially
since the here is a line that is nearly as long just above.

The real fix is to reduce the number of levels of indentation
to something sane.
I suspect that use of continue, break and return will help.

The other line length changes have much the same problem
but not as sever.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2022-02-17 02:58:51

by Phillip Potter

[permalink] [raw]
Subject: Re: [PATCH v2 15/15] staging: r8188eu: correct long line warnings near prior DBG_88E calls

On Wed, Feb 16, 2022 at 10:01:18AM +0000, David Laight wrote:
> From: Phillip Potter
> > Sent: 16 February 2022 01:07
> >
> > Where it is possible (without out-of-patch-series-scope large scale
> > refactoring), correct code to remove checkpatch warnings about lines
> > that are too long, also correcting operator spacing where appropriate
> > for these lines as well. These warnings occur mostly due to so many
> > DBG_88E removals and parentheses tweaks etc. being adjacent to such
> > long lines, which are therefore included in the resultant diff.
> ...
>
> Somewhere my copy of this seems to have got its tabs deleted.
> I blame outlook :-)
>
> > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > index ddc3a2c8aaca..d68611ef22f8 100644
> > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> > @@ -382,7 +382,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> > if (protocol == ETH_P_IP) {
> > struct iphdr *iph = (struct iphdr *)(skb->data + ETH_HLEN);
> >
> > -if (((unsigned char *)(iph) + (iph->ihl<<2)) >= (skb->data + ETH_HLEN + skb->len))
> > +if (((unsigned char *)(iph) + (iph->ihl << 2)) >= (skb->data + ETH_HLEN + skb->len))
>
> You can delete at least three sets of () from that line.
>
> > return -1;
> >
> > switch (method) {
> > @@ -451,7 +451,11 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
> > pOldTag = (struct pppoe_tag *)__nat25_find_pppoe_tag(ph, ntohs(PTT_RELAY_SID));
> > if (pOldTag) { /* if SID existed, copy old value and delete it */
> > old_tag_len = ntohs(pOldTag->tag_len);
> > -if (old_tag_len+TAG_HDR_LEN+MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN > sizeof(tag_buf))
> > +if (old_tag_len +
> > + TAG_HDR_LEN +
> > + MAGIC_CODE_LEN +
> > + RTL_RELAY_TAG_LEN >
> > + sizeof(tag_buf))
> > return -1;
>
> That change really doesn't help readability at all.
> There isn't much point shortening it that much like that, especially
> since the here is a line that is nearly as long just above.
>
> The real fix is to reduce the number of levels of indentation
> to something sane.
> I suspect that use of continue, break and return will help.
>
> The other line length changes have much the same problem
> but not as sever.
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

Dear David,

Thank you for your feedback, and yes I totally agree - this patch was
more for procedure's sake to quieten the checkpatch warnings, but I was
in two minds about whether I should include it.

The indentation level is absolutely what is the problem here, but it is
arguably not in scope for this particular patch set given these are
pre-existing lines that have the issue. Certainly needs fixing though
for sure - just that this is more substantial and worthy of a separate
patch set in my opinion.

Looks like I need to do V3 anyway as I missed an unused-but-set warning
in patch 5 of the series. I may therefore drop this patch in V3 and
perhaps work on the indentation problem more generally.

Regards,
Phil