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
