Re: [virtio-comment] [PATCH 1/1] ccw: ccw payload description

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Tue, 21 Jan 2014 10:53:49 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Jan 21, 2014 at 10:37:26AM +1030, Rusty Russell wrote:
> > Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > > On Mon, 20 Jan 2014 11:17:17 +0200
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >
> > >> On Mon, Jan 20, 2014 at 02:28:00PM +1030, Rusty Russell wrote:
> > >> > We'll just have to live with the lack of padding, and let's try not to
> > >> > add any more packed-requiring structures.  If we deprecate
> > >> > virtio-balloon, the rest will be contained in CCW.
> > >> > 
> > >> > Cheers,
> > >> > Rusty.
> > >> 
> > >> For CCW, how about my idea of adding optional padding explicitly?
> > >> In the compatibility section, we can explicitly specify the
> > >> the length.
> > >> Drivers can use offsetof(padding) and avoid packed attribute.
> > >
> > > Since Rusty does not like the tables, this looks like a good idea.
> > 
> > Surely making the padding optional makes it more complex, for no real
> > gain?  It's not that much simpler in practice, is it?
> > 
> > This patch just removes the __attribute__((packed)) where it's useless,
> > and notes explicitly where it's required:

(...)

> I was trying to avoid the non-portable __attribute__((packed))
> replacing it with portable offsetof(..., padding)
> everywhere. It only has to be optional for the
> transitional devices. But if Thomas is fine with
> __attribute__((packed)) now, that's fine with me too.
> 
> I do think we need a general comment explaining the
> packed directive here.
> 
> E.g.
> 
> Many device and driver in-memory structure layouts
> are documented using the C struct syntax.
> In this case no extra padding is assumed within the structure.
> To stress this, cases where common C compilers are known to insert extra
> padding within structures are tagged using the GNU C __attribute__((packed))
> syntax.

Do we want to add a general statement regarding our usage of C structs?

Basically, we can't predict if and where a compiler will insert
padding, and different compilers on different architectures might pad
in different ways. So I'd propose to state that all our C structs are
assumed to be packed with no extra padding appended, even if we take
care to introduce no new structures where a common C compiler would
insert padding.

(We may want to specify our le32, ... usage in the same paragraph.)