Re: byteorder helpers and void * (was: Re: [PATCH 01/21] lib: add byteorder helpers for the aligned case)

On Tue, 24 Jun 2008, Harvey Harrison wrote:
On Tue, 2008-06-24 at 14:54 +0200, Geert Uytterhoeven wrote:
Document the fact that void * passed in needs to be 16-bit aligned?

Why not let it just take a __le16 *? Because in many use-cases the pointer just
points to an array of bytes?

For the unaligned case, e.g. get_unaligned_le16(), I can understand a bit the
rationale about using void * (a typical use-case is accessing a little endian
16-bit value in the middle of an arrays of bytes).

However, a disadvantage is that you remove the ability of the compiler to check
for using the wrong accessor in a (packed for the unaligned case) struct, e.g.

struct {
u8 pad;
__le16 val; /* 16-bit value */
} __attribute ((packed)) s;

x = get_unaligned_le32(&s.val); /* oops, 32-bit access */


I'm starting to come around to the typechecking argument. This would
also be a chance to fix the argument ordering in put_analigned_XXXX
that was noticed by others. As there are already some existing users
in-tree, we could transition gradually by:

1) Introduce typed versions of get/put_unaligned_XXXX, that implies the
byteswap better:
u16 load_unaligned_le16(__le16 *)
void store_unaligned_le16(__le16 *, u16)

OK

Then the aligned helpers could be:
le16_to_cpup -> aligned equivalent of load_unaligned_le16
store_le16(__le16 *, u16)

Implemented as (to allow constant folding)
#define store_le16(ptr, val) (*(__le16 *)(ptr) = cpu_to_le16((u16)(val)))

Again, no typechecking in store_le16(), due to the explicit cast.

I noticed there's also a __get_unaligned_le(), which uses compile-time
detection of the pointer time, to make sure the correct accessor is used.
Do you intend this to be used by generic code? It's function name starts
with double underscore, indicating otherwise.

It is not meant for generic use, it is just there as a helper for each
arch to wire up it's get_unaligned() macro depending on its endianness,
so each arch doesn't wire up its own version that may or may not have
the size checking.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre
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@xxxxxxxxxxx
Internet: http://www.sony-europe.com/

Sony Technology and Software Centre Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis 293-0376800-10 GEBA-BE-BB

Relevant Pages

  • Additional question about two-dimensional indexing
    ... I have a 2D array of cells, and so far I could not find a way to instantiated it, rather than ... And correspondingly is there a way to index wire in a 2D manner. ... Section: Multiple-Bit Wire Connections ... I want groups of 4 cells at the time to have the same input, so there will be 32 signals, xgoing to 128 cells. ...
    (comp.cad.cadence)
  • Re: Store serial data inside an array
    ... I want to design a module that performs a simple operation: ... synchronous serial data inside an array. ... wire pixel_out; ... Any hint about the correct solution (and most important: ...
    (comp.lang.verilog)
  • Additional Question: two-dimensional arrays
    ... I have a 2D array of cells, and so far I could not find a way to instantiated it, rather than ... And correspondingly is there a way to index wire in a 2D manner. ... Section: Multiple-Bit Wire Connections ... I want groups of 4 cells at the time to have the same input, so there will be 32 signals, xgoing to 128 cells. ...
    (comp.cad.cadence)
  • Store serial data inside an array
    ... I want to design a module that performs a simple operation: ... synchronous serial data inside an array. ... wire pixel_out; ... Any hint about the correct solution (and most important: ...
    (comp.lang.verilog)
  • Re: Sorting
    ... Most in Europe use a semicolon. ... Svilen Pachedzhiev wrote: ... > Fill the rest of the array by substituting the proper T ...
    (microsoft.public.excel)