Johannes Berg <johan...@sipsolutions.net> writes:
> From: Johannes Berg <johannes.b...@intel.com>
>
> It seems like a historic accident that these return unsigned char *,
> and in many places that means casts are required, more often than not.
>
> Make these functions (skb_put, __skb_put and pskb_put) return void *
> and remove all the casts across the tree, adding a (u8 *) cast only
> where the unsigned char pointer was used directly, all done with the
> following spatch:
>
> @@
> expression SKB, LEN;
> typedef u8;
> identifier fn = { skb_put, __skb_put };
> @@
> - *(fn(SKB, LEN))
> + *(u8 *)fn(SKB, LEN)
>
> @@
There seem to be a large number of places where the char pointer was
used directly. Not that I have any strong opinion either way, but adding
lots of ugly casts like this seems to contradict the whole purpose of
this change?:
> if (info->rx_count == 0) {
> diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
> index 8165ef2fe877..be2d431aa366 100644
> --- a/drivers/bluetooth/bt3c_cs.c
> +++ b/drivers/bluetooth/bt3c_cs.c
> @@ -282,7 +282,7 @@ static void bt3c_receive(struct bt3c_info *info)
>
> __u8 x = inb(iobase + DATA_L);
>
> - *skb_put(info->rx_skb, 1) = x;
> + *(u8 *)skb_put(info->rx_skb, 1) = x;
> inb(iobase + DATA_H);
> info->rx_count--;
>
That does not look any better in my eyes, and there are ... what?
... hundreds of them?
No complaints about the other skb_* changes you are doing, Those are
nice cleanups. But skb_put is different IMHO.
Bjørn
