[PATCH net-next] net: caif: use skb helpers instead of open-coding them

[PATCH net-next] net: caif: use skb helpers instead of open-coding them

[Jann Horn]

Use existing skb_put_data() and skb_trim() instead of open-coding them,
with the skb_put_data() first so that logically, `skb` still contains the
data to be copied in its data..tail area when skb_put_data() reads it.
This change on its own is a cleanup, and it is also necessary for potential
future integration of skbuffs with things like KASAN.

Signed-off-by: Jann Horn <jannh@google.com>
---
 net/caif/cfpkt_skbuff.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
index 38c2b7a890dd..37ac5ca0ffdf 100644
--- a/net/caif/cfpkt_skbuff.c
+++ b/net/caif/cfpkt_skbuff.c
@@ -319,16 +319,12 @@ struct cfpkt *cfpkt_append(struct cfpkt *dstpkt,
 		if (tmppkt == NULL)
 			return NULL;
 		tmp = pkt_to_skb(tmppkt);
-		skb_set_tail_pointer(tmp, dstlen);
-		tmp->len = dstlen;
-		memcpy(tmp->data, dst->data, dstlen);
+		skb_put_data(tmp, dst->data, dstlen);
 		cfpkt_destroy(dstpkt);
 		dst = tmp;
 	}
-	memcpy(skb_tail_pointer(dst), add->data, skb_headlen(add));
+	skb_put_data(dst, add->data, skb_headlen(add));
 	cfpkt_destroy(addpkt);
-	dst->tail += addlen;
-	dst->len += addlen;
 	return skb_to_pkt(dst);
 }
 
@@ -359,13 +355,11 @@ struct cfpkt *cfpkt_split(struct cfpkt *pkt, u16 pos)
 	if (skb2 == NULL)
 		return NULL;
 
+	skb_put_data(skb2, split, len2nd);
+
 	/* Reduce the length of the original packet */
-	skb_set_tail_pointer(skb, pos);
-	skb->len = pos;
+	skb_trim(skb, pos);
 
-	memcpy(skb2->data, split, len2nd);
-	skb2->tail += len2nd;
-	skb2->len += len2nd;
 	skb2->priority = skb->priority;
 	return skb_to_pkt(skb2);
 }
-- 
2.21.0.rc0.258.g878e2cd30e-goog

Re: [PATCH net-next] net: caif: use skb helpers instead of open-coding them

[Jann Horn]

On Thu, Feb 14, 2019 at 10:35 PM Jann Horn <jannh@google.com> wrote:
> Use existing skb_put_data() and skb_trim() instead of open-coding them,
> with the skb_put_data() first so that logically, `skb` still contains the
> data to be copied in its data..tail area when skb_put_data() reads it.
> This change on its own is a cleanup, and it is also necessary for potential
> future integration of skbuffs with things like KASAN.

The maintainer's email address (dmitry.tarnyagin@lockless.no) bounces
with a "553 5.3.0 <dmitry.tarnyagin@lockless.no>... No such user
here". His second address that shows up in the git log,
dmitry.tarnyagin@stericsson.com, also doesn't exist:

$ dig +short stericsson.com MX
10 mxb-00178001.gslb.pphosted.com.
10 mxa-00178001.gslb.pphosted.com.
$ nc -vC mxa-00178001.gslb.pphosted.com 25
Connection to mxa-00178001.gslb.pphosted.com 25 port [tcp/smtp] succeeded!
220 mx07-00178001.pphosted.com ESMTP mfa-m0046668
HELO thejh.net
250 mx07-00178001.pphosted.com Hello {...} (may be forged), pleased to meet you
MAIL FROM: <test@thejh.net>
250 2.1.0 Sender ok
RCPT TO: <dmitry.tarnyagin@stericsson.com>
550 5.1.1 User Unknown

But his third address from a commit he made in 2011,
abi.dmitryt@gmail.com, does exist, so I'm CC'ing that now.

@Dmitry Tarnyagin: Do you still consider yourself to be the maintainer
of the CAIF subsystem? MAINTAINERS currently lists it as "Supported"
by you, which is defined as "Someone is actually paid to look after
this".

> Signed-off-by: Jann Horn <jannh@google.com>
> ---
>  net/caif/cfpkt_skbuff.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
> index 38c2b7a890dd..37ac5ca0ffdf 100644
> --- a/net/caif/cfpkt_skbuff.c
> +++ b/net/caif/cfpkt_skbuff.c
> @@ -319,16 +319,12 @@ struct cfpkt *cfpkt_append(struct cfpkt *dstpkt,
>                 if (tmppkt == NULL)
>                         return NULL;
>                 tmp = pkt_to_skb(tmppkt);
> -               skb_set_tail_pointer(tmp, dstlen);
> -               tmp->len = dstlen;
> -               memcpy(tmp->data, dst->data, dstlen);
> +               skb_put_data(tmp, dst->data, dstlen);
>                 cfpkt_destroy(dstpkt);
>                 dst = tmp;
>         }
> -       memcpy(skb_tail_pointer(dst), add->data, skb_headlen(add));
> +       skb_put_data(dst, add->data, skb_headlen(add));
>         cfpkt_destroy(addpkt);
> -       dst->tail += addlen;
> -       dst->len += addlen;
>         return skb_to_pkt(dst);
>  }
>
> @@ -359,13 +355,11 @@ struct cfpkt *cfpkt_split(struct cfpkt *pkt, u16 pos)
>         if (skb2 == NULL)
>                 return NULL;
>
> +       skb_put_data(skb2, split, len2nd);
> +
>         /* Reduce the length of the original packet */
> -       skb_set_tail_pointer(skb, pos);
> -       skb->len = pos;
> +       skb_trim(skb, pos);
>
> -       memcpy(skb2->data, split, len2nd);
> -       skb2->tail += len2nd;
> -       skb2->len += len2nd;
>         skb2->priority = skb->priority;
>         return skb_to_pkt(skb2);
>  }
> --
> 2.21.0.rc0.258.g878e2cd30e-goog
>

Re: [PATCH net-next] net: caif: use skb helpers instead of open-coding them

[David Miller]

From: Jann Horn <jannh@google.com>
Date: Thu, 14 Feb 2019 22:35:47 +0100

> Use existing skb_put_data() and skb_trim() instead of open-coding them,
> with the skb_put_data() first so that logically, `skb` still contains the
> data to be copied in its data..tail area when skb_put_data() reads it.
> This change on its own is a cleanup, and it is also necessary for potential
> future integration of skbuffs with things like KASAN.
> 
> Signed-off-by: Jann Horn <jannh@google.com>

Applied, thanks.

Feel free to send me a MAINTAINERS patch that either updates the
bouncing email or removes it altogether.  If the listed maintainer
bounces and won't reply when you ask them if they are still
maintaining things it is valid to just remove them.

Thanks.