Re: [Qemu-devel] [RFC PATCH 3/4] block: VHDX block driver support

[Kevin Wolf]

On Tue, Feb 19, 2013 at 08:26:35AM -0500, Jeff Cody wrote:
> On Tue, Feb 19, 2013 at 10:20:40AM +0100, Stefan Hajnoczi wrote:
> > On Mon, Feb 18, 2013 at 06:03:31PM -0500, Jeff Cody wrote:
> > > +#include "qemu/crc32c.h"
> > > +#include "block/vhdx.h"
> > > +
> > > +#define vhdx_nop(x) do { (void)(x); } while (0)
> > > +
> > > +/* Help macros to copy data from file buffers to header
> > > + * structures, with proper endianness.  These help avoid
> > > + * using packed structs */
> > 
> > Why not use packed structs?  Then you don't need memcpy/endianness
> > macros.  Abusing *_to_cpu() to go both "to" and "from" CPU endianness
> > is ugly.
> 
> Packed structs are (IMO) worse, and I think should be avoided wherever
> possible.  Depending on the platform and structure, it can lead to
> unaligned data access, which is either just slower (x86) or worse
> (e.g. some arm variants).  Maybe these are all defined well in the
> spec, so as to be mostly alignment proof from most architectures, but
> I figured it just as well to avoid their usage since we have to touch
> each field for alignment (well, I guess only on BE architectures in
> the end).

The whole point of packed structs is to enable the unaligned data
access, so yes, you're right with that. But in my opinion declaring the
struct packed and letting the compiler deal with unaligned data is
clearly superior to doing it manually. I mean, this is the kind of
things that compilers are for.

Kevin