On Wed, May 29, 2013 at 02:28:32PM +0200, Paolo Bonzini wrote:
> Il 29/05/2013 14:16, Michael S. Tsirkin ha scritto:
> >>>> > >> If you really want to use offsetof like this you're
> >>>> > >> going to need to decorate the structs with QEMU_PACKED.
> >> >
> >>> > > Nope.
> >>> > > These structs are carefully designed not to have any padding.
> >> >
> >> > ...on every compiler and OS combination that QEMU builds for?
> > Yes. All the way back to EGCS and before.
> > GCC has been like this for many many years.
>
> I would still prefer to have QEMU_PACKED (or __attribute((__packed__))
> in the kernel).
>
> >>> > > And if there was a bug and there was some padding, we still
> >>> > > can't fix it with PACKED because this structure
> >>> > > is used to interact with the guest code which does not
> >>> > > have the packed attribute.
> >> >
> >> > The guest code has to use a set of structure offsets and
> >> > sizes which is fixed by the virtio ABI -- how it implements
> >> > this is up to the guest (and if it misimplements it that is
> >> > a guest bug and not our problem).
>
> On the other hand, encouraging both userspace and guest to reuse a
> single set of headers means that the bad offset becomes a spec bug more
> than a guest bug.
Heh
> > Deviating from driver in random ways is an endless source
> > of hard to debug issues, and it's a very practical
> > consideration because virtio spec is constantly
> > being extended (unlike hardware which is mostly fixed).
>
> Agreed---but the driver should use __attribute__((__packed__)) too.
>
> Paolo
For these specific structures I don't mind, we never dereference them
anyway. If you do dereference a structure, using packed is generally
a mistake. In particular because GCC generates code that is
much slower.
You can also get nasty surprises e.g.
struct foo {
char a;
int b;
} __attribute__((packed));
struct foo foo;
int *a = &foo.a;
Last line compiles fine but isn't legal C.
So I think there are two cases with packed:
1. it does not change logical behaviour but results in bad code
generated
2. it does change logical behaviour and leads to unsafe code
There aren't many good reasons to use packed: if you
must treat unaligned data, best to use constants.
--
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
