lkml.org 
[lkml]   [2020]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 13/17] staging: wfx: fix endianness of the field 'len'
Hi Jerome,

On Mon, May 11, 2020 at 5:53 PM Jerome Pouiller
<Jerome.Pouiller@silabs.com> wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
>
> The struct hif_msg is received from the hardware. So, it declared as
> little endian. However, it is also accessed from many places in the
> driver. Sparse complains about that:
>
> drivers/staging/wfx/bh.c:88:32: warning: restricted __le16 degrades to integer
> drivers/staging/wfx/bh.c:88:32: warning: restricted __le16 degrades to integer
> drivers/staging/wfx/bh.c:93:32: warning: restricted __le16 degrades to integer
> drivers/staging/wfx/bh.c:93:32: warning: cast to restricted __le16
> drivers/staging/wfx/bh.c:93:32: warning: restricted __le16 degrades to integer
> drivers/staging/wfx/bh.c:121:25: warning: incorrect type in argument 2 (different base types)
> drivers/staging/wfx/bh.c:121:25: expected unsigned int len
> drivers/staging/wfx/bh.c:121:25: got restricted __le16 [usertype] len
> drivers/staging/wfx/hif_rx.c:27:22: warning: restricted __le16 degrades to integer
> drivers/staging/wfx/hif_rx.c:347:39: warning: incorrect type in argument 7 (different base types)
> drivers/staging/wfx/hif_rx.c:347:39: expected unsigned int [usertype] len
> drivers/staging/wfx/hif_rx.c:347:39: got restricted __le16 const [usertype] len
> drivers/staging/wfx/hif_rx.c:365:39: warning: incorrect type in argument 7 (different base types)
> drivers/staging/wfx/hif_rx.c:365:39: expected unsigned int [usertype] len
> drivers/staging/wfx/hif_rx.c:365:39: got restricted __le16 const [usertype] len
> drivers/staging/wfx/./traces.h:195:1: warning: incorrect type in assignment (different base types)
> drivers/staging/wfx/./traces.h:195:1: expected int msg_len
> drivers/staging/wfx/./traces.h:195:1: got restricted __le16 const [usertype] len
> drivers/staging/wfx/./traces.h:195:1: warning: incorrect type in assignment (different base types)
> drivers/staging/wfx/./traces.h:195:1: expected int msg_len
> drivers/staging/wfx/./traces.h:195:1: got restricted __le16 const [usertype] len
> drivers/staging/wfx/debug.c:319:20: warning: restricted __le16 degrades to integer
> drivers/staging/wfx/secure_link.c:85:27: warning: restricted __le16 degrades to integer
> drivers/staging/wfx/secure_link.c:85:27: warning: restricted __le16 degrades to integer

Thanks for your patch!

> In order to make Sparse happy and to keep access from the driver easy,
> this patch declare 'len' with native endianness.
>
> On reception of hardware data, this patch takes care to do byte-swap and
> keep Sparse happy.

Which means sparse can no longer do any checking on the field,
and new bugs may/will creep in in the future, unnoticed.

> --- a/drivers/staging/wfx/hif_api_general.h
> +++ b/drivers/staging/wfx/hif_api_general.h
> @@ -23,7 +23,10 @@
> #define HIF_COUNTER_MAX 7
>
> struct hif_msg {
> - __le16 len;
> + // len is in fact little endian. However, it is widely used in the
> + // driver, so we declare it in native byte order and we reorder just
> + // before/after send/receive it (see bh.c).
> + u16 len;

While there's a small penalty associated with always doing the conversion
on big-endian platforms, it will probably be lost in the noise anyway.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

\
 
 \ /
  Last update: 2020-05-12 09:44    [W:0.249 / U:0.532 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site