[Endian] Unaligned fields in packed structs

Dear All, One of my all-time worst bugs was due to calling htonl() on an unaligned field in a packed struct. It worked in debug mode but not in production code, and there were no compiler warnings about the broken alignment requirement. So I thought I'd try this: inline void inplace_byteswap(uint32_t& val) { val = (val>>24) | ((val>>8) & 0x0000ff00) | ((val<<8) & 0x00ff0000) | (val<<24); } struct S { uint16_t a; uint32_t b; } __attribute__ ((packed)); int main() { S s; s.b = 0x12345678; byteswap(s.b); } This fails to compile: "error: cannot bind packed field ‘s.S::b’ to ‘uint32_t& {aka unsigned int&}’". So it's good that it doesn't just fail unpredictably at runtime like my old nightmare, but it's not ideal. If instead I write: inline uint32_t byteswap(uint32_t val) { return (val>>24) | ((val>>8) & 0x0000ff00) | ((val<<8) & 0x00ff0000) | (val<<24); } ... s.b = byteswap(s.b); ... then of course it does work; the unaligned accesses don't "escape" from where the compiler knows about them and can apply fixups. Anyway, before I tried this I was already wondering about the choice of function signatures: void endian::reorder(T&); // in-place byteswap void endian::reorder(T src, T& dest); // out-of-place byteswap What is the rationale (or precedent) for these and not T endian::reorder(T); ? If sin(theta) returns its result, why shouldn't reorder(val) do so too? Regards, Phil.

...
Anyway, before I tried this I was already wondering about the choice of function signatures:
void endian::reorder(T&); // in-place byteswap void endian::reorder(T src, T& dest); // out-of-place byteswap
What is the rationale (or precedent) for these and not
T endian::reorder(T);
?
The endian/conversion.hpp interfaces are very immature compared to the endian/integers.hpp interfaces, so it is well worth taking a hard look at your questions. The unconditional interfaces and conditional interfaces were designed as a set, so they do things in a similar way. If native_to_big, as an example, was supplied only as: T native_to_big(T x); then the code for big-endian systems wouldn't just be a no-op. A copy, no matter how fast, is a slower than a no-op. Of course a really smart compiler might optimize the do-nothing copy away. Also, some real applications deal with records of many fields, so you get long strings of: reorder(rec.f1); reorder(rec.f2); ...etc. It is a bit wordier to have to write: rec.f1 = reorder(rec.f1); rec.f2 = reorder(rec.f2); ...etc. I'll run some timings tomorrow to see how actual release build timings differ. --Beman

On 09/07/2011 03:08 AM, Beman Dawes wrote:
If native_to_big, as an example, was supplied only as:
T native_to_big(T x);
then the code for big-endian systems wouldn't just be a no-op. A copy, no matter how fast, is a slower than a no-op. Of course a really smart compiler might optimize the do-nothing copy away.
Assuming T is a POD, if the function is inlined, the copy will be elided. I wouldn't think it really requires a particularly smart compiler.

On Wed, Sep 7, 2011 at 4:44 AM, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
On 09/07/2011 03:08 AM, Beman Dawes wrote:
If native_to_big, as an example, was supplied only as:
T native_to_big(T x);
then the code for big-endian systems wouldn't just be a no-op. A copy, no matter how fast, is a slower than a no-op. Of course a really smart compiler might optimize the do-nothing copy away.
Assuming T is a POD, if the function is inlined, the copy will be elided. I wouldn't think it really requires a particularly smart compiler.
Looking at the generated code, that is definitely the case for the Microsoft compiler, at least in some initial tests. I'll take a look at GCC in the next day or so. Thanks, --Beman

On Tue, Sep 6, 2011 at 7:48 PM, Phil Endecott <spam_from_boost_dev@chezphil.org> wrote:
... I was already wondering about the choice of function signatures:
void endian::reorder(T&); // in-place byteswap void endian::reorder(T src, T& dest); // out-of-place byteswap
What is the rationale (or precedent) for these and not
T endian::reorder(T);
?
If sin(theta) returns its result, why shouldn't reorder(val) do so too?
I've implemented your suggestion, run various tests, looked at the generated code, and tried various uses cases. Bottom line: I think you are right and will change conversion.hpp accordingly. Thanks, --Beman
participants (3)
-
Beman Dawes
-
Mathias Gaunard
-
Phil Endecott