Re: endianness and sparse warnings

[Harvey Harrison]

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 */

> 
> 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_.

Cheers,

Harvey

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/