lkml.org 
[lkml]   [2008]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: endianness and sparse warnings
On Fri, 29 Aug 2008, Harvey Harrison wrote:
> On Fri, 2008-08-29 at 16:54 +0200, Geert Uytterhoeven wrote:
> > With `make C=1 CF="-D__CHECK_ENDIAN__"', you can let sparse check for bad
> > handling of endian-annotated data.
> >
> > Unfortunately several of the accessors for endian-annotated data do not cause
> > sparse warnings.
>
> I'll try and give some background on why the unaligned versions are implemented
> this way.
>
> The get_unaligned helpers were meant to replace two kinds of use (using le16 as
> an example)
>
> char *ptr;
>
> 1 - le16_to_cpu(get_unaligned((__le16 *)ptr))
> 2 - u16 val = ptr[0] | (ptr[1] << 8)
>
> The places where 1 was replaced with the unaligned helpers would have been fine
> with an annotated version as it already had the cast to a proper type.
>
> The places where 2 was replaced would have required a new cast to __le16 *.
>
> Lots of places that were using 2 are drivers that have some data area pointed
> to by a char * and they are grabbing values from there at known offsets,
> for these users, the need for extra casting was quite ugly and it was known
> exactly how many bytes and in what endianness you are reading as it is
> right in the function name so I thought it would be ok to omit the annotation
> on the parameter.
>
> u16 foo, bar;
> char *my_data;
>
> foo = get_unaligned_le16((__le16 *)my_data); /* if unaligned helpers were annotated */
> bar = get_unaligned_le16(my_data); /* current version */

Indeed, if you have a void/char *, this works, and there's not much annotation
you can do here.

But please consider this case:

struct xxx {
u8 a;
__le16 b;
} __attribute__ ((packed));

struct xxx *p;
u16 foo = get_unaligned_le16(&p->b);

Here you could have typechecking.
Note that a `get_unaligned_le()' that handles different sizes automatically
would work.

> > Summarized:
> > - [bl]e{16,32,64}_to_cpu() is OK
> > - [bl]e{16,32,64}_to_cpup() (aka get_aligned_[bl]e{16,32,64}() ;-) is OK
> > - get_unaligned_[bl]e{16,32,64} is not OK
> > - __get_unaligned_[bl]e() is partially OK, as long as you don't use it on
> > non-annotated data, but
> > o it's meant for internal use only
> > o it incorrectly causes sparse warnings when assigning the resulting
> > value to a non-annotated variable
>
> Almost... __get_unaligned_le16 etc are _never_ to be used...as some arches
> choose to use memmove-based implementations, and on arches where unaligned
> access is OK, they don't exist _at_all_.

So perhaps we want a public get_unaligned_[bl]e() for the case above?

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
\
 
 \ /
  Last update: 2008-09-01 11:27    [W:0.037 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site