lkml.org 
[lkml]   [2021]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] staging: rtl8188eu: Replace a custom function with crc32_le()
Date
On Tuesday, July 13, 2021 2:50:47 PM CEST Greg Kroah-Hartman wrote:
> On Thu, Jul 01, 2021 at 03:38:09PM +0200, Fabio M. De Francesco wrote:
> > Use crc32_le in place of the custom getcrc32. This change makes GCC
> > to warn about incorrect castings to the restricted type __le32, but
> > they can be safely ignored because crc32_le calculates bitwise
> > little-endian Ethernet AUTODIN II CRC32.
> >
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> >
> > drivers/staging/rtl8188eu/core/rtw_security.c | 22 ++++---------------
> > 1 file changed, 4 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8188eu/core/rtw_security.c
> > b/drivers/staging/rtl8188eu/core/rtw_security.c index
1b2cb6196463..5f010cb66970
> > 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_security.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_security.c
> > @@ -111,21 +111,6 @@ static void crc32_init(void)
> >
[...]
>
> Why are you casting a native endian return value to a little endian
> pointer?
>
Actually, I was not sure whether or not crc32_le() returns native or little
endian values:

(1) That "_le" in crc32_le() made me think it calculates little endian values
and the documentation of the function is a little misleading where it affirms
that "[crc32_le()] calculates *bitwise
little-endian* Ethernet AUTODIN II CRC32".

(2) Furthermore, I took commit b97fad10de387 as a model for my patch. You
applied that commit and it makes exactly the same change that I make in my
patch. I still cannot understand why you took that. Maybe I'm missing
something that I can't still see. What is it?
>
> Are you _SURE_ that is correct?
>
To summarize, I thought that crc32_le is some way returning an LE value in an
array of four u8 (aka an unsigned integer).

Now I suppose I have to change "*((__le32 *)crc) = ~crc32_le(~0, payload,
length);" to "*((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)".

Thanks,

Fabio
> We can not just ignore warnings, they are there for a reason. Or if
> not, then fix the code up to not have the warnings, but I can't take
> this as-is, sorry.
>
> thanks,
>
> greg k-h



\
 
 \ /
  Last update: 2021-07-13 19:54    [W:0.886 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site